Skip to content

feat(desktop): add LiteLLM-aware connection diagnostics - #464

Open
Lxr-max wants to merge 2 commits into
OpenCoworkAI:mainfrom
Lxr-max:feat/litellm-diagnostics
Open

Lxr-max wants to merge 2 commits into
OpenCoworkAI:mainfrom
Lxr-max:feat/litellm-diagnostics

Conversation

@Lxr-max

@Lxr-max Lxr-max commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Connection tests and model discovery for the LiteLLM Gateway preset now explain the failure instead of showing a generic HTTP or network error. A 401 or 403 tells the user to set the LiteLLM master or virtual key, or to turn off keyless mode when the proxy requires a key. A 404 tells them to check that the base URL ends with /v1. That hint mentions port 4000 only when the URL is loopback (localhost, 127.0.0.1, or ::1). A connection refusal tells them to start the LiteLLM proxy. The hints are translated in en, zh-CN, es, and pt-BR.

A target counts as LiteLLM when the preset id is litellm, the name or provider id says LiteLLM, or the base URL host or path says LiteLLM. Port 4000 alone does not.

This finishes the diagnostics acceptance criterion that #463 left open. The preset itself is unchanged, and LiteLLM is still not bundled.

Type of change

  • Bug fix
  • New feature
  • Refactor (no behavior change)
  • Documentation
  • Build / CI / tooling
  • Breaking change

Linked issue

Closes #208

Together with #463, this covers the acceptance criteria on #208: the LiteLLM Gateway preset, proxy-key and keyless setup, LiteLLM hints on connection test and model discovery, and model discovery through the existing OpenAI-compatible path (with manual model entry in the same modal). Title generation, the agent runtime, and the generic connection-diagnostic panel are not part of #208.

Checklist

  • I checked the linked issue / relevant context before starting
  • pnpm lint && pnpm typecheck && pnpm test passes locally
  • Added/updated tests for the change
  • Added a changeset (pnpm changeset) if user-visible
  • Updated docs if behavior changed (the hints are the user-facing copy; no separate doc change)

PRINCIPLES §5b

  • Compatibility: additive. Other providers keep the existing generic hints. The optional presetId field on config:v1:test-endpoint is ignored unless it is litellm. Saved providers are recognized from the name, the provider id, or a host or path that says LiteLLM, not from port 4000.
  • Upgradeability: no on-disk schema change. A saved LiteLLM provider is still a normal custom OpenAI-compatible entry. The new hintKey is an optional field on existing error responses.
  • No bloat: no new dependencies. One preset's failure copy, wired through the existing connection-test and model-list errors.
  • Elegance: hint selection lives in one helper. Connection test, endpoint probe, and model discovery all attach that key, and the renderer translates it.

Screenshots / recordings (UI changes)

No new screen. The LiteLLM sentence replaces the generic connection-test and discovery failure text in the add-provider modal, the provider-row test toast, the model list under a provider, and the model switcher. Covered by the connection-probe tests and the locale strings.

When a connection test or model discovery fails for the LiteLLM Gateway preset, explain the usual cause: set the master or virtual key or turn off keyless mode on 401, check that the base URL ends with /v1 and that the proxy is on port 4000 on 404, and start the LiteLLM proxy when the connection is refused.
@github-actions github-actions Bot added docs Documentation area:desktop apps/desktop (Electron shell, renderer) labels Oct 4, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Nit] The notFound diagnostic hardcodes the local default port, which is misleading for remote LiteLLM deployments — the LiteLLM preset explicitly supports remote gateways (the pre-existing hint says “…edit it for remote deployments”), yet the new 404 copy tells every user to ensure the proxy “is running on port 4000” (packages/i18n/src/locales/en.json:704, plus the matching es/pt-BR/zh-CN entries). For a remote gateway that check is irrelevant.
    Suggested fix: keep the /v1 assertion and soften the port clause (e.g. “…ends with /v1, or that a local proxy is running on port 4000”) in all four locale files, then update the toContain('port 4000') / toContain('4000') assertions in packages/i18n/src/i18n.test.ts:184.

  • [Nit] Loopback-port detection can mislabel a non-LiteLLM gateway — isLiteLlmDefaultPort (packages/shared/src/litellm-hints.ts:49) treats any http(s)://localhost|127.0.0.1|[::1]:4000 target as LiteLLM (packages/shared/src/litellm-hints.ts:22), so a user running a different OpenAI-compatible server on port 4000 gets LiteLLM-specific advice such as “Start the LiteLLM proxy” on a transport error. The heuristic is documented and intentional (packages/shared/src/litellm-hints.ts:1), so this is polish only: either drop the port-only branch (identity already comes from presetId/name/providerId) or phrase that copy conditionally.

Questions

  • I could not load the linked issue bodies in this run, so I could not verify the closure claim against #208 / #463. Can you confirm that diagnostics is the last open acceptance criterion on #208 and that nothing else (title generation, agent runtime, diagnostics beyond connection-test/model-list) is still expected there? If any part remains, switch the link to Refs #208.

Summary

Review mode: initial

No blockers or majors found. The change is additive: presetId is an optional, validated field (apps/desktop/src/main/connection-ipc.ts:54), the new hintKey is optional on existing error responses, unsupported presetId values are rejected (parseLiteLlmPresetId), and non-LiteLLM providers still get the previous generic copy — the connection test, degraded-inference probe, endpoint probe, and model-list paths are all wired consistently, and the renderer falls back to the existing hint/message via connectionFailureText (apps/desktop/src/renderer/src/lib/connection-failure-text.ts:2). No new dependencies, no on-disk schema change, and a changeset covering shared/i18n/desktop is included, so the PRINCIPLES §5b checklist holds. Tests cover the new helper, the IPC paths, and the four locales. The two Nits above are copy/polish and do not block merge.

Residual observations (non-blocking):

  • In ModelSwitcher/RowModelSelector the component keeps models non-null after a failure, so the failure hint persists and will not refetch on reopen; this matches pre-existing behavior and is not introduced here.
  • AddCustomProviderModal’s discovery-failed branch calls t(hintKey) directly while the test-failure branch uses connectionFailureText; both are correct, just slightly inconsistent.

Testing

  • Reviewed existing coverage: packages/shared/src/litellm-hints.test.ts, apps/desktop/src/main/connection-ipc.test.ts (connection test, degraded probe, saved-provider test, model discovery, and the “other gateway stays quiet” cases), apps/desktop/src/renderer/src/lib/connection-failure-text.test.ts, and packages/i18n/src/i18n.test.ts.
  • Suggested addition: a renderer test that a hintKey on a failed listForProvider response renders the translated hint in ModelSwitcher/RowModelSelector, since those translation paths are currently only exercised indirectly.
  • Not run (automation).

Open-CoDesign Bot

Recognize a LiteLLM gateway from the preset id, name, provider id, or a host or path that says LiteLLM. Port 4000 alone no longer counts. The 404 hint mentions that port only when the base URL is loopback.

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

area:desktop apps/desktop (Electron shell, renderer) docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature]: 增加 LiteLLM Gateway 一等预设 / Add a first-class LiteLLM gateway preset

2 participants