Skip to content

feat: add openai-compatible provider for custom OpenAI API endpoints#79

Open
simcop2387 wants to merge 1 commit into
Gsync:mainfrom
simcop2387:main
Open

feat: add openai-compatible provider for custom OpenAI API endpoints#79
simcop2387 wants to merge 1 commit into
Gsync:mainfrom
simcop2387:main

Conversation

@simcop2387

Copy link
Copy Markdown

Disclosure: coding done by Qwen3.6-27B running locally to my machines.

This adds support for any openai compatible api such as LM Studio, Llama.cpp, DwarfStar/DS4, VLLM, LiteLLM, Azure OpenAI (not tested but should work), etc.

I've reviewed the changes myself and they look reasonable and pretty isolated to just the places that make sense but I'm not sure if there's anything that isn't in line with the project's style guidelines or anything.

Commit message from Qwen:

Add support for any OpenAI-compatible LLM API (e.g. LM Studio, Ollama with OpenAI API, LiteLLM, Azure OpenAI). Users configure a base URL and optional API key via the settings UI, or via OPENAI_COMPAT_BASE_URL and OPENAI_COMPAT_API_KEY env vars.

  • Added OPENAI_COMPATIBLE to AiProvider enum
  • Added registry entry with base-url credential type
  • Added dual-credential resolution (base URL + optional API key)
  • Added models API route and verifier
  • Added second input in settings UI for API key
  • Added helper functions for credential resolution

Add support for any OpenAI-compatible LLM API (e.g. LM Studio, Ollama with
OpenAI API, LiteLLM, Azure OpenAI). Users configure a base URL and optional
API key via the settings UI, or via OPENAI_COMPAT_BASE_URL and
OPENAI_COMPAT_API_KEY env vars.

- Added OPENAI_COMPATIBLE to AiProvider enum
- Added registry entry with base-url credential type
- Added dual-credential resolution (base URL + optional API key)
- Added models API route and verifier
- Added second input in settings UI for API key
- Added helper functions for credential resolution
@Gsync

Gsync commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Review

Thanks for the contribution — the registry-driven design (reusing PROVIDER_REGISTRY + modelsEndpoint + parseModelsResponse instead of hardcoding a new UI path) is the right way to extend this system, and the change is reasonably scoped (219/-27 across 13 files).

That said, a few things need addressing before this can be merged.

Contribution guideline compliance (CONTRIBUTING.md)

  • Wrong base branch — this PR targets main. Guidelines require all PRs to target dev, never main (main is only updated by maintainers via versioned merges from dev). Please retarget/rebase onto dev.
  • Branch naming — the head branch is main on the fork, not a feat/...-prefixed branch as the naming convention asks for.
  • No tests added/updated — none of the 13 changed files are test files. At minimum, the dual-credential verifier branching ({baseURL, apiKey} vs plain string) and the new credential-resolution path are worth covering.
  • No CI run reported on this PR (likely pending approval for a first-time contributor) — please confirm npm run lint and npm test pass locally, per the pre-PR checklist.
  • PR description — doesn't follow the template; no "Testing" section describing how this was verified.

Code quality

  1. Dead/duplicate code in src/actions/apiKey.actions.ts: resolveApiKeyRecord, getOpenaiCompatibleBaseUrl, and getOpenaiCompatibleApiKey are never called anywhere in the codebase. They duplicate logic that already exists and is actually used — resolveApiKey in src/lib/api-key-resolver.ts, which the rest of this PR correctly relies on in providers.ts and the new models route. Recommend deleting these three.

  2. PROVIDER_VERIFIERS type loosened to any in provider-registry.server.ts — the signature changed from (key: string) => ... to (key: any) => ... for all six verifiers (openai, openrouter, deepseek, ollama, gemini, rapidapi), not just the new one. This drops type checking across the board to accommodate one provider's {baseURL, apiKey} shape. A narrower union like (key: string | { baseURL: string; apiKey?: string }) => ... would keep the other five type-safe.

  3. Vestigial factory entryPROVIDER_FACTORIES["openai-compatible"] only throws and is unreachable in practice, since getModel() special-cases and returns before ever calling factory() for this provider. It exists solely to dodge the earlier if (!factory) throw guard. Works, but consider checking the provider special-case before the factory lookup instead.

  4. Inconsistent indentation in the new JSX in ApiKeySettings.tsx (the "API Key" badge block and the following ollama block) — looks like it needs a pass through the project formatter.

  5. getDefaultModelForProvider returns "" for OPENAI_COMPATIBLE in src/lib/scraper/runner.ts. Reasonable in principle (no universal default model for arbitrary endpoints), but if an automation runs with this provider and no model explicitly selected, modelName silently becomes "" and is passed straight into createOpenAI(...)("") with no early validation — this will surface as a confusing downstream fetch error rather than a clear "select a model" message.

Why two ApiKeyProvider entries (openai-compatible / openai-compatible-key)?

Flagging this since it's a bit unusual: the ApiKey table stores one credential value per (userId, provider) pair, but this feature needs two independent secrets (base URL + optional API key). Rather than a schema change, the PR stores the API key under a second, synthetic "provider" row (openai-compatible-key) that isn't a real provider — it has no PROVIDER_REGISTRY entry and isn't in AI_PROVIDERS, it's purely a namespace hack to get a second value out of the existing table via resolveApiKey. It works, and avoids a migration, but it does mean ApiKeyProvider's type now contains an entry that isn't an actual AI provider, and every place that iterates providers has to know to skip it. A dedicated column on ApiKey (with a migration) would be the cleaner long-term fix, but the current approach is a reasonable pragmatic trade-off if avoiding schema churn here is intentional.

Security

No new gaps: the new /api/ai/openai-compatible/models route follows the same pattern as the existing openai/models route (relies on global middleware auth rather than an explicit 401 check — not a regression). No schema changes. Sensitive/non-sensitive handling is correct (base URL plaintext like Ollama's, optional API key encrypted like the other cloud providers).

Summary

Solid, well-integrated feature that follows the existing registry pattern faithfully. Before merging: retarget to dev with a feat/-prefixed branch, drop the three unused functions, tighten the verifier type, add test coverage, and fix the JSX indentation.


🤖 Review by Claude Code

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants