Skip to content

Comments

Unify OpenAI-compatible providers and custom provider flow#930

Open
PeterDaveHello wants to merge 2 commits intoChatGPTBox-dev:masterfrom
PeterDaveHello:refactor/openai-provider-platform
Open

Unify OpenAI-compatible providers and custom provider flow#930
PeterDaveHello wants to merge 2 commits intoChatGPTBox-dev:masterfrom
PeterDaveHello:refactor/openai-provider-platform

Conversation

@PeterDaveHello
Copy link
Member

@PeterDaveHello PeterDaveHello commented Feb 24, 2026

User description

This PR unifies OpenAI-compatible API execution into a shared core + provider registry,
and completes the custom provider workflow in API Modes.

Included changes

  • Consolidated OpenAI-compatible request logic into shared execution path
  • Unified provider resolution/secret lookup for built-in and custom providers
  • Added config-driven custom provider flow in API Modes
  • Kept GPT-5/OpenAI token param behavior (max_completion_tokens where required)
  • Hardened migration for legacy configs/sessions (provider IDs, keys, custom URL mapping)
  • Fixed settings/key update edge cases and stream completion signaling consistency

Compatibility

  • No intended breaking behavior for existing users
  • Existing configs are migrated with backward-compatible handling
  • Existing OpenAI-compatible providers continue to work under unified config management

Validation

  • npm run lint passed
  • npm run build passed

GitHub Copilot PR summary

This pull request introduces a major refactor to how OpenAI-compatible and custom API providers are managed and configured. The changes unify the handling of various OpenAI-compatible APIs (including custom, Ollama, DeepSeek, Moonshot, ChatGLM, OpenRouter, AIML, and legacy GPT Completion APIs) under a single provider system, and add a robust migration and normalization layer for user configuration. This will make it easier to add new providers, manage secrets, and ensure backward compatibility with legacy config data.

Key changes include:

Unification and Refactor of API Provider Handling

  • Consolidated all OpenAI-compatible API providers (custom, Ollama, DeepSeek, Moonshot, ChatGLM, OpenRouter, AIML, GPT Completion, etc.) to be handled by a single generateAnswersWithOpenAICompatibleApi function, replacing multiple specific imports and execution branches in src/background/index.mjs. This simplifies the codebase and centralizes provider logic. [1] [2] [3] [4]

  • Added isUsingOpenAICompatibleApiSession utility to determine if a session should be routed through the unified OpenAI-compatible API handler.

Configuration Schema and Migration

  • Introduced a configuration schema versioning system and a comprehensive migration function (migrateUserConfig) in src/config/index.mjs. This function normalizes provider IDs, migrates legacy secrets, deduplicates and renames custom providers, and ensures all config data is up-to-date and consistent.

  • Updated the default config to include new fields: customOpenAIProviders, providerSecrets, and configSchemaVersion.

Popup UI and Provider Management

  • Refactored the API modes section in the popup UI (src/popup/sections/ApiModes.jsx) to use the new provider registry and normalization logic. Added helpers for provider ID normalization, uniqueness, and base URL sanitization.

  • Ensured that UI state and config updates are consistent with the new provider structure, and that legacy custom providers are handled gracefully.


References:

  • Unified provider imports and execution: [1] [2] [3] [4]
  • Added isUsingOpenAICompatibleApiSession:
  • Config migration and normalization:
  • Default config updates:
  • Popup/provider UI refactor:

PR Type

Enhancement


Description

  • Unified OpenAI-compatible provider handling into shared core module

  • Added config migration system with provider registry and secrets management

  • Implemented custom provider workflow in API Modes UI

  • Consolidated multiple provider-specific API handlers into single execution path


Diagram Walkthrough

flowchart LR
  A["Multiple Provider APIs<br/>openai, custom, ollama, etc."] -->|consolidate| B["Unified OpenAI<br/>Compatible Core"]
  C["Legacy Config<br/>scattered keys"] -->|migrate| D["Provider Registry<br/>+ Secrets Map"]
  E["API Modes UI"] -->|create/select| F["Custom Providers"]
  B --> G["Shared Request<br/>Execution"]
  D --> G
  F --> D
Loading

File Walkthrough

Relevant files
Refactoring
4 files
index.mjs
Consolidate provider dispatch into unified handler             
+19/-68 
GeneralPart.jsx
Unify API key management via provider registry                     
+127/-123
custom-api.mjs
Delegate to unified OpenAI-compatible core                             
+11/-92 
openai-api.mjs
Route all providers through unified handler                           
+85/-158
Enhancement
7 files
index.mjs
Add config migration and provider registry system               
+373/-3 
ApiModes.jsx
Implement custom provider creation and selection UI           
+199/-43
openai-compatible-core.mjs
New shared core for all OpenAI-compatible APIs                     
+159/-0 
provider-registry.mjs
New provider registry with resolution logic                           
+318/-0 
init-session.mjs
Normalize API mode during session initialization                 
+6/-2     
wrappers.mjs
Apply API mode normalization to port listener                       
+6/-1     
model-name-convert.mjs
Add API mode normalization and provider ID support             
+40/-5   

Summary by CodeRabbit

  • New Features
    • Added custom OpenAI-compatible provider management system enabling creation and configuration of custom AI provider endpoints
    • Implemented automatic configuration migration for legacy provider settings
    • Unified handling across multiple OpenAI-compatible API providers for improved consistency

Consolidate OpenAI-compatible API request handling into a single shared
module and route background dispatch through provider registry lookup.
This removes duplicated streaming/parsing logic from openai-api and
custom-api while keeping existing behavior.

Add config migration to preserve existing API keys and custom mode
entries by mapping them into providerSecrets and custom provider
records. Keep legacy fallbacks for apiMode customUrl/custom apiKey to
avoid user-visible regressions during rollout.

Normalize apiMode objects at runtime and compare selection using stable
identity fields so migrated and legacy session data continue to match
correctly.
Extend API Modes editing so custom model entries can bind to a
providerId and create a new OpenAI-compatible provider in the same flow
with only provider name and base URL.

Unify API key editing in General settings by resolving the currently
selected OpenAI-compatible provider and writing secrets into the new
providerSecrets map, while still syncing legacy key fields for backward
compatibility.

Preserve legacy custom URL behavior for legacy provider mode and clear
apiMode.customUrl when users switch to a registered provider so provider
registry URLs are applied correctly.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

This PR consolidates multiple OpenAI-compatible API implementations (ChatGPT API, GPT Completion, custom, Ollama, ChatGLM, Moonshot, DeepSeek, OpenRouter, AIML) into a unified provider registry and core request handler. Adds configuration migration for legacy provider settings and refactors provider management UI.

Changes

Cohort / File(s) Summary
Provider Registry & Core Infrastructure
src/services/apis/provider-registry.mjs, src/services/apis/openai-compatible-core.mjs
New unified OpenAI-compatible provider system: provider registry with built-in and custom provider templates, resolution logic for provider selection/endpoints, and centralized core handler for streaming SSE requests. Replaces per-provider implementations with configurable, reusable core logic.
API Integration Refactoring
src/services/apis/openai-api.mjs, src/services/apis/custom-api.mjs
Updated API wrappers to delegate to unified OpenAI-compatible core; removed bespoke SSE and prompt handling logic; added new public entry generateAnswersWithOpenAICompatibleApi for provider-aware request construction and delegation.
Config System & Migration
src/config/index.mjs
Introduces configuration migration layer with normalization helpers for legacy provider keys; extends defaultConfig with customOpenAIProviders, providerSecrets, and configSchemaVersion; migrateUserConfig persists schema upgrades and rehomes legacy API keys to new provider IDs.
Session & API Mode Normalization
src/services/init-session.mjs, src/services/wrappers.mjs, src/utils/model-name-convert.mjs
Added normalizeApiMode utility to validate and normalize API mode objects; session initialization and port listener now apply normalization; updated API mode shape to include providerId and enhanced comparison logic for provider-aware selection.
Background & Routing
src/background/index.mjs
Introduced isUsingOpenAICompatibleApiSession helper to aggregate OpenAI-compatible model checks; refactored execution flow to route applicable sessions through unified OpenAI-compatible provider instead of handling each specialized API separately; removed individual API imports in favor of consolidated approach.
Provider Management UI
src/popup/sections/ApiModes.jsx, src/popup/sections/GeneralPart.jsx
Added custom OpenAI provider management UI with provider selector, new provider creation form (name/base URL), and provider-aware API key handling; replaced multiple provider-specific input blocks with unified provider-based approach; updated balance retrieval and conditional rendering to use provider registry.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Suggested labels

Review effort 3/5

Poem

🐰 Providers unified, a rabbit's delight,
Registry brings order to the API night,
Legacy keys find homes anew,
OpenAI-compatible paths shine bright and true,
One core to rule them all—what a sight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Unify OpenAI-compatible providers and custom provider flow' directly summarizes the main architectural change: consolidating OpenAI-compatible API execution and implementing a unified custom provider workflow.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @PeterDaveHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the extension's architecture for managing AI API providers. By unifying OpenAI-compatible API interactions and introducing a comprehensive provider registry with robust configuration migration, the changes aim to improve maintainability, simplify the addition of new providers, and enhance the user experience for custom API configurations. This also ensures better consistency in how API keys and endpoints are handled across the application.

Highlights

  • Unified OpenAI-compatible API Handling: Consolidated all OpenAI-compatible API execution logic into a single shared core function, replacing multiple individual API implementations and simplifying the codebase.
  • Enhanced Provider Registry and Configuration: Introduced a robust provider registry for both built-in and custom OpenAI-compatible providers, centralizing their management, secret lookup, and configuration.
  • Comprehensive Configuration Migration: Implemented a new configuration schema versioning system and a migration function to normalize provider IDs, migrate legacy API keys, deduplicate custom providers, and ensure backward compatibility for existing user settings.
  • Improved Custom Provider Workflow in UI: Refactored the API Modes section in the popup UI to support the new provider registry, allowing users to define and manage custom providers with unique IDs, base URLs, and API key handling.
  • Streamlined API Key Management: Centralized API key storage and retrieval through providerSecrets, ensuring that API keys are consistently managed across different providers and API modes, and handling updates gracefully.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • src/background/index.mjs
    • Removed direct imports and calls for individual OpenAI-compatible APIs (e.g., Custom, Ollama, Moonshot, DeepSeek, ChatGLM, OpenRouter, AIML, GPT Completion).
    • Introduced a new isUsingOpenAICompatibleApiSession utility function to determine if a session should use the unified API handler.
    • Replaced multiple conditional API calls with a single call to generateAnswersWithOpenAICompatibleApi for all OpenAI-compatible providers.
  • src/config/index.mjs
    • Added providerId field to the default API mode configuration.
    • Introduced new configuration fields: customOpenAIProviders, providerSecrets, and configSchemaVersion.
    • Implemented a migrateUserConfig function to handle schema versioning, normalize provider IDs, migrate legacy API keys to providerSecrets, and deduplicate/rename custom providers.
    • Updated getUserConfig to apply the migrateUserConfig function and persist any necessary migrations.
  • src/popup/sections/ApiModes.jsx
    • Removed CustomApiKeyGroups and CustomUrlGroups imports, relying on the new provider registry.
    • Added LEGACY_CUSTOM_PROVIDER_ID constant for backward compatibility.
    • Introduced defaultProviderDraft for creating new custom providers.
    • Added utility functions normalizeProviderId, createProviderId, normalizeBaseUrl, and sanitizeApiModeForSave for UI-related provider management.
    • Updated useLayoutEffect to fetch customProviders from the new registry.
    • Refactored the onSaveEditing function to handle new custom provider creation and update existing API modes with provider IDs and secrets.
    • Modified the API mode editing component to include a provider selector for choosing existing custom providers or creating new ones.
    • Adjusted the logic for setting editingApiMode and providerSelector when editing an existing API mode or adding a new one.
  • src/popup/sections/GeneralPart.jsx
    • Removed imports for individual API model checks (e.g., isUsingOpenAiApiModel, isUsingChatGLMApiModel, isUsingMoonshotApiModel, etc.).
    • Introduced LEGACY_API_KEY_FIELD_BY_PROVIDER_ID mapping for secret migration.
    • Added buildProviderSecretUpdate function to manage API key updates across providerSecrets and legacy fields, and to sync API keys in customApiModes and apiMode.
    • Updated getBalance function to use the resolved OpenAI-compatible provider's base URL and API key.
    • Modified the API key input field to dynamically display the API key from the selectedProviderApiKey and update it using buildProviderSecretUpdate.
    • Removed specific API key input fields for ChatGLM, Moonshot, DeepSeek, Ollama, OpenRouter, and AIML, centralizing API key management.
  • src/services/apis/custom-api.mjs
    • Removed extensive custom API logic, including SSE fetching and message parsing.
    • Replaced the custom API implementation with a call to the new generateAnswersWithOpenAICompatible core function, passing relevant parameters.
  • src/services/apis/openai-api.mjs
    • Removed direct SSE fetching and message parsing logic from generateAnswersWithGptCompletionApi and generateAnswersWithChatgptApiCompat.
    • Introduced normalizeBaseUrl and resolveModelName utility functions.
    • Added touchOllamaKeepAlive function for Ollama provider-specific keep-alive requests.
    • Refactored generateAnswersWithGptCompletionApi and generateAnswersWithChatgptApiCompat to utilize the new generateAnswersWithOpenAICompatible core function.
    • Added generateAnswersWithOpenAICompatibleApi as a unified entry point for all OpenAI-compatible providers, resolving provider details and calling the core function.
  • src/services/apis/openai-compatible-core.mjs
    • Added new file implementing generateAnswersWithOpenAICompatible.
    • Implemented a generic SSE fetching and message parsing mechanism for OpenAI-compatible APIs.
    • Provided helper functions buildHeaders, buildMessageAnswer, and hasFinished for consistent API interaction.
    • Dynamically constructs request bodies for both 'chat' and 'completion' endpoint types, handling conversation context and token parameters.
  • src/services/apis/provider-registry.mjs
    • Added new file to manage OpenAI-compatible API providers.
    • Defined DEFAULT_CHAT_PATH, DEFAULT_COMPLETION_PATH, and LEGACY_KEY_BY_PROVIDER_ID constants.
    • Defined BUILTIN_PROVIDER_TEMPLATE for various pre-configured providers (OpenAI, DeepSeek, Moonshot, OpenRouter, AIML, ChatGLM, Ollama, Legacy Custom).
    • Defined OPENAI_COMPATIBLE_GROUP_TO_PROVIDER_ID mapping for legacy group names.
    • Implemented utility functions like getModelNamePresetPart, resolveProviderIdFromLegacyModelName, isLegacyCompletionModelName, toStringOrEmpty, trimSlashes, ensureLeadingSlash, and joinUrl.
    • Provided functions to build built-in providers (buildBuiltinProviders), normalize custom providers (normalizeCustomProvider), get custom providers (getCustomOpenAIProviders), and get all providers (getAllOpenAIProviders).
    • Implemented functions to resolve provider ID for a session (resolveProviderIdForSession), endpoint type (resolveEndpointTypeForSession), get provider by ID (getProviderById), and retrieve provider secrets (getProviderSecret).
    • Added resolveOpenAICompatibleRequest as a central function to determine the correct provider, endpoint, URL, and API key for a given session.
  • src/services/init-session.mjs
    • Imported normalizeApiMode from model-name-convert.mjs.
    • Applied normalizeApiMode to the apiMode during session initialization to ensure consistency.
  • src/services/wrappers.mjs
    • Imported normalizeApiMode from model-name-convert.mjs.
    • Applied normalizeApiMode to session.apiMode within the registerPortListener to ensure API mode objects are consistently normalized.
  • src/utils/model-name-convert.mjs
    • Added providerId field to the modelNameToApiMode function's return object for custom API modes.
    • Introduced a new normalizeApiMode function to ensure API mode objects have consistent structure and default values.
    • Updated apiModeToModelName to use normalizeApiMode for robust handling of API mode objects.
    • Updated getApiModesFromConfig to use normalizeApiMode when processing custom API modes and to correctly handle Ollama model names.
    • Modified isApiModeSelected to use normalizeForCompare (which leverages normalizeApiMode) for more reliable comparison of API mode objects.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
SSRF credential exfiltration

Description: resolveUrlFromProvider() allows session.apiMode.customUrl (for customApiModelKeys) to
fully override the request destination while getProviderSecret() can still supply a real
provider API key (e.g., OpenAI) from config.providerSecrets, enabling a
malicious/compromised config (or manually edited storage) to exfiltrate a user's API key
by sending an authenticated request to an attacker-controlled URL (SSRF/credential leak).
provider-registry.mjs [270-318]

Referred Code
function resolveUrlFromProvider(provider, endpointType, config, session) {
  if (!provider) return ''

  const apiModeCustomUrl =
    endpointType === 'chat' &&
    session?.apiMode &&
    typeof session.apiMode === 'object' &&
    session.apiMode.groupName === 'customApiModelKeys'
      ? toStringOrEmpty(session.apiMode.customUrl).trim()
      : ''
  if (apiModeCustomUrl) return apiModeCustomUrl

  if (endpointType === 'completion') {
    if (provider.completionsUrl) return provider.completionsUrl
    if (provider.completionsPath) return joinUrl(provider.baseUrl, provider.completionsPath)
  } else {
    if (provider.chatCompletionsUrl) return provider.chatCompletionsUrl
    if (provider.chatCompletionsPath) return joinUrl(provider.baseUrl, provider.chatCompletionsPath)
  }

  if (provider.id === 'legacy-custom-default') {


 ... (clipped 28 lines)
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing URL validation: getBalance can build a billing URL from an empty/undefined base URL
(selectedProvider?.baseUrl || config.customOpenAiApiUrl), potentially resulting in an
invalid or relative fetch without graceful handling.

Referred Code
const getBalance = async () => {
  const openAiApiUrl = selectedProvider?.baseUrl || config.customOpenAiApiUrl
  const response = await fetch(`${openAiApiUrl}/dashboard/billing/credit_grants`, {
    headers: {
      'Content-Type': 'application/json',
      Authorization: `Bearer ${selectedProviderApiKey}`,
    },
  })
  if (response.ok) setBalance((await response.json()).total_available.toFixed(2))
  else {
    const billing = await checkBilling(selectedProviderApiKey, openAiApiUrl)
    if (billing && billing.length > 2 && billing[2]) setBalance(`${billing[2].toFixed(2)}`)
    else openUrl('https://platform.openai.com/account/usage')

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Sensitive debug logs: The new unified core logs raw SSE messages and conversation history via console.debug,
which can include user content and other sensitive data in logs.

Referred Code
  console.debug('conversation history', { content: session.conversationRecords })
  port.postMessage({ answer: null, done: true, session: session })
}

await fetchSSE(requestUrl, {
  method: 'POST',
  signal: controller.signal,
  headers: buildHeaders(apiKey, extraHeaders),
  body: JSON.stringify(requestBody),
  onMessage(message) {
    console.debug('sse message', message)
    if (finished) return
    if (message.trim() === '[DONE]') {
      finish()
      return
    }
    let data
    try {
      data = JSON.parse(message)
    } catch (error) {
      console.debug('json error', error)


 ... (clipped 1 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
No audit logging: The new migration path writes sensitive configuration/secrets to local storage without any
audit trail context (who/when/what/outcome), making post-incident reconstruction
difficult.

Referred Code
export async function getUserConfig() {
  const options = await Browser.storage.local.get(Object.keys(defaultConfig))
  const { migrated, dirty } = migrateUserConfig(options)
  if (dirty) {
    const payload = {
      customChatGptWebApiUrl: migrated.customChatGptWebApiUrl,
      customApiModes: migrated.customApiModes,
      customOpenAIProviders: migrated.customOpenAIProviders,
      providerSecrets: migrated.providerSecrets,
      configSchemaVersion: migrated.configSchemaVersion,
    }
    if (migrated.apiMode !== undefined) payload.apiMode = migrated.apiMode
    await Browser.storage.local.set(payload)
  }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Provider error exposed: The SSE onError path throws an Error built from resp.json() or status text, which may
propagate provider-specific details to user-facing surfaces depending on upstream
handling.

Referred Code
async onError(resp) {
  port.onMessage.removeListener(messageListener)
  port.onDisconnect.removeListener(disconnectListener)
  if (resp instanceof Error) throw resp
  const error = await resp.json().catch(() => ({}))
  throw new Error(!isEmpty(error) ? JSON.stringify(error) : `${resp.status} ${resp.statusText}`)
},

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Weak URL validation: Custom provider baseUrl/endpoint derivation relies on minimal normalization (trim/trailing
slash removal and regex heuristics) without stricter validation (scheme/host
allowlisting), which may permit unsafe or unintended request targets depending on
extension threat model.

Referred Code
function normalizeBaseUrl(value) {
  return String(value || '')
    .trim()
    .replace(/\/+$/, '')
}

function sanitizeApiModeForSave(apiMode) {
  const nextApiMode = { ...apiMode }
  if (nextApiMode.groupName !== 'customApiModelKeys') {
    nextApiMode.providerId = ''
    nextApiMode.apiKey = ''
    return nextApiMode
  }
  if (!nextApiMode.providerId) nextApiMode.providerId = LEGACY_CUSTOM_PROVIDER_ID
  return nextApiMode
}

export function ApiModes({ config, updateConfig }) {
  const { t } = useTranslation()
  const [editing, setEditing] = useState(false)
  const [editingApiMode, setEditingApiMode] = useState(defaultApiMode)


 ... (clipped 86 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
The migration logic is overly complex

The migrateUserConfig function is over 300 lines long and overly complex,
creating a high risk of corrupting user data. Consider a simpler, staged
migration to improve maintainability and reduce risk.

Examples:

src/config/index.mjs [796-1083]
function migrateUserConfig(options) {
  const migrated = { ...options }
  let dirty = false

  if (migrated.customChatGptWebApiUrl === 'https://chat.openai.com') {
    migrated.customChatGptWebApiUrl = 'https://chatgpt.com'
    dirty = true
  }

  const providerSecrets =

 ... (clipped 278 lines)

Solution Walkthrough:

Before:

// src/config/index.mjs
function migrateUserConfig(options) {
  const migrated = { ...options };
  let dirty = false;

  // ... ~300 lines of interwoven logic ...

  // Migrate legacy secrets to providerSecrets
  for (const [legacyKey, providerId] of Object.entries(LEGACY_SECRET_KEY_TO_PROVIDER_ID)) { ... }

  // Normalize and deduplicate custom provider IDs
  // ... complex logic with sets and maps ...

  // Migrate customApiModes, creating new providers on the fly
  for (const apiMode of customApiModes) {
    if (apiMode.groupName !== 'customApiModelKeys') { ... } 
    else {
      // ... create new providers from customUrl ...
      // ... migrate apiMode keys to providerSecrets ...
    }
  }

  // Migrate selected apiMode
  if (migrated.apiMode) { ... }

  return { migrated, dirty };
}

After:

// A simpler, staged migration approach
// src/config/index.mjs

function migrateLegacySecrets(config) {
  // Only handles migrating top-level API keys to providerSecrets.
  // ...
  return newConfig;
}

function migrateCustomProviders(config) {
  // Only handles normalizing custom provider definitions.
  // ...
  return newConfig;
}

function migrateApiModes(config) {
  // Only handles linking apiModes to the new provider IDs.
  // May flag ambiguous cases for user review instead of auto-resolving.
  // ...
  return newConfig;
}

function migrateUserConfig(options) {
    let migrated = { ...options };
    migrated = migrateLegacySecrets(migrated);
    migrated = migrateCustomProviders(migrated);
    migrated = migrateApiModes(migrated);
    // ... other simple migrations ...
    return { migrated, dirty };
}
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies the high complexity and risk of the monolithic migrateUserConfig function, which is critical for preventing user data loss and ensuring future maintainability.

Medium
General
Build billing URL from provider request

To correctly query billing information for all providers, derive the billing
endpoint's base URL from the fully resolved requestUrl instead of using the
provider's baseUrl.

src/popup/sections/GeneralPart.jsx [206-212]

-const openAiApiUrl = selectedProvider?.baseUrl || config.customOpenAiApiUrl
-const response = await fetch(`${openAiApiUrl}/dashboard/billing/credit_grants`, {
+// extract billing base from the provider request URL
+const billingBase = selectedProviderRequest.requestUrl.replace(
+  /(\/v1\/chat\/completions|\/v1\/completions).*$/,
+  ''
+)
+const response = await fetch(`${billingBase}/dashboard/billing/credit_grants`, {
   headers: {
     'Content-Type': 'application/json',
     Authorization: `Bearer ${selectedProviderApiKey}`,
   },
 })
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly points out that the billing check should derive its base URL from the resolved request URL, not the provider's base URL, to correctly handle providers with non-standard URL structures.

Medium
Remove legacy API keys after migration

After migrating legacy API keys to the new providerSecrets map, explicitly
remove the old top-level keys from browser storage to avoid redundant data.

src/config/index.mjs [1100-1101]

 if (migrated.apiMode !== undefined) payload.apiMode = migrated.apiMode
 await Browser.storage.local.set(payload)
 
+const legacyKeysToRemove = Object.keys(LEGACY_SECRET_KEY_TO_PROVIDER_ID).filter(
+  (key) => migrated[key] !== undefined,
+)
+if (legacyKeysToRemove.length > 0) {
+  await Browser.storage.local.remove(legacyKeysToRemove)
+}
+
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that legacy API keys are not removed from storage after migration and proposes a clean solution to remove them, improving data hygiene and preventing potential confusion.

Low
Possible issue
Use customModelApiUrl for custom completions

For the legacy custom provider, ensure completion requests use
config.customModelApiUrl before falling back to config.customOpenAiApiUrl to
correctly route to custom endpoints.

src/services/apis/provider-registry.mjs [290-298]

 if (provider.id === 'legacy-custom-default') {
   if (endpointType === 'completion') {
-    return `${trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com')}/v1/completions`
+    const customUrl = toStringOrEmpty(config.customModelApiUrl).trim()
+    return customUrl
+      ? `${trimSlashes(customUrl)}/v1/completions`
+      : `${trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com')}/v1/completions`
   }
   return (
     toStringOrEmpty(config.customModelApiUrl).trim() ||
     'http://localhost:8000/v1/chat/completions'
   )
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that legacy completion requests were incorrectly using customOpenAiApiUrl instead of customModelApiUrl, and provides a fix to respect the user's custom endpoint for completions.

Medium
  • More

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request consolidates OpenAI-compatible API providers (OpenAI, DeepSeek, Moonshot, ChatGLM, OpenRouter, AIML, Ollama, and custom providers) into a unified provider system with a shared execution core and provider registry. The changes enable config-driven custom provider management and implement comprehensive migration logic for backward compatibility.

Changes:

  • Introduced a unified provider registry (provider-registry.mjs) and shared OpenAI-compatible API execution core (openai-compatible-core.mjs) to consolidate previously scattered provider logic
  • Implemented comprehensive config migration system to normalize provider IDs, migrate legacy secrets to new providerSecrets map, handle custom URL to provider mapping, and ensure data consistency
  • Enhanced API Modes UI to support creating and selecting custom OpenAI-compatible providers with automatic ID generation and validation

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/services/apis/provider-registry.mjs New provider registry system for resolving provider configurations, secrets, and API endpoints for all OpenAI-compatible providers
src/services/apis/openai-compatible-core.mjs New unified execution path for OpenAI-compatible API requests with streaming support
src/services/apis/openai-api.mjs Refactored to use the new unified execution core, removing duplicate code and adding unified provider routing
src/services/apis/custom-api.mjs Simplified to delegate to the new shared core, removing 80+ lines of duplicate logic
src/config/index.mjs Added comprehensive migrateUserConfig function to normalize provider IDs, migrate secrets, deduplicate providers, and ensure backward compatibility
src/popup/sections/GeneralPart.jsx Updated API key input handling to use unified provider secret management via buildProviderSecretUpdate
src/popup/sections/ApiModes.jsx Enhanced to support creating and managing custom OpenAI providers with validation and automatic ID generation
src/utils/model-name-convert.mjs Added normalizeApiMode function and updated comparison logic to include providerId field
src/services/wrappers.mjs Updated to normalize apiMode when initializing sessions
src/services/init-session.mjs Updated to normalize apiMode during session initialization
src/background/index.mjs Consolidated provider routing through new isUsingOpenAICompatibleApiSession helper and unified API entry point

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

},
async onStart() {},
async onEnd() {
if (!finished) port.postMessage({ done: true })
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

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

The onEnd handler unconditionally sends a done: true message even if the stream has already finished. This differs from other API handlers like azure-openai-api.mjs which always send the message. While this is likely not harmful, it creates an inconsistent pattern across the codebase. Consider either:

  1. Always sending the message for consistency with other handlers, or
  2. Documenting why this handler needs different behavior.
Suggested change
if (!finished) port.postMessage({ done: true })
// Always send a final done message for consistency with other API handlers
port.postMessage({ done: true })

Copilot uses AI. Check for mistakes.
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly refactors the API mode handling by unifying OpenAI-compatible providers and introducing a custom provider flow. The changes centralize API logic, improve configuration management through schema versioning and migration, and enhance the UI for custom provider creation and selection. The refactoring reduces code duplication and sets a solid foundation for future extensibility. Overall, the changes are well-structured and address key areas for maintainability and user experience.

I am having trouble creating individual review comments. Click here to see my feedback.

src/background/index.mjs (437-458)

medium

The removal of the isUsingCustomModel(session) block and its associated generateAnswersWithCustomApi calls is a direct result of the unification efforts. This simplifies the executeApi function by delegating custom API handling to the new generateAnswersWithOpenAICompatibleApi function, reducing conditional logic and improving code clarity.

src/background/index.mjs (566-568)

medium

The introduction of isUsingOpenAICompatibleApiSession(session) and the call to generateAnswersWithOpenAICompatibleApi consolidates the logic for various OpenAI-compatible providers. This is a core part of the unification, making the executeApi function more streamlined and extensible. All previously separate API calls for OpenAI, Moonshot, ChatGLM, DeepSeek, Ollama, OpenRouter, and AIML are now routed through this single entry point.

src/config/index.mjs (550-556)

medium

The addition of providerId, customOpenAIProviders, providerSecrets, and configSchemaVersion to the defaultConfig is crucial for the new configuration schema and migration system. providerId allows custom API modes to link to specific providers, while customOpenAIProviders and providerSecrets manage custom provider details and their API keys. configSchemaVersion enables future-proof migration logic.

src/config/index.mjs (729-733)

medium

The getUserConfig function has been updated to incorporate the migrateUserConfig logic. This ensures that any legacy configurations are automatically migrated to the new schema upon loading, providing backward compatibility and consistency across user settings. The dirty flag correctly triggers a save if migration occurs.

src/popup/sections/ApiModes.jsx (31)

medium

The addition of providerId to defaultApiMode is necessary to support the new provider registry system. This allows API modes to be explicitly linked to a specific provider, whether built-in or custom, enabling more granular control and clearer configuration.

src/popup/sections/ApiModes.jsx (98)

medium

The useLayoutEffect now includes config.customOpenAIProviders as a dependency. This ensures that the UI re-renders and updates the list of custom providers whenever the underlying configuration for custom OpenAI providers changes, maintaining UI consistency with the backend data.

src/popup/sections/ApiModes.jsx (121-134)

medium

The persistApiMode function is a new utility that centralizes the logic for saving API mode configurations. It handles updating customApiModes and optionally customOpenAIProviders, and ensures that the currently selected API mode is also updated if it's being edited. This improves code organization and reduces duplication.

src/popup/sections/ApiModes.jsx (136-195)

medium

The onSaveEditing function has been completely refactored to support the new custom provider creation and selection flow. It now handles the creation of new providers, assigns provider IDs, and manages the clearing of API keys when switching providers. This is a critical piece of the custom provider workflow, ensuring data integrity and proper linking between API modes and providers.

src/popup/sections/ApiModes.jsx (267-288)

medium

This new UI section allows users to select an existing custom provider or create a new one. The providerSelector state manages the selection, and the options are dynamically populated from customProviders. This is a key enhancement for the custom provider workflow, making it user-friendly.

src/popup/sections/ApiModes.jsx (290-305)

medium

This new block provides input fields for creating a new custom provider, including its name and base URL. This directly supports the custom provider creation flow, allowing users to define new OpenAI-compatible endpoints directly within the UI.

src/popup/sections/GeneralPart.jsx (12)

medium

The removal of isUsingOpenAiApiModel and isUsingChatGLMApiModel from imports reflects the consolidation of these checks into the new isUsingOpenAICompatibleProvider utility. This simplifies the import list and centralizes the logic for identifying OpenAI-compatible API usage.

src/popup/sections/GeneralPart.jsx (96-105)

medium

The LEGACY_API_KEY_FIELD_BY_PROVIDER_ID constant maps provider IDs to their legacy API key field names in the configuration. This is essential for the migration and unified secret management, allowing the system to correctly identify and update API keys for various providers.

src/popup/sections/GeneralPart.jsx (107-181)

medium

The buildProviderSecretUpdate function is a critical new utility for managing API keys. It handles updating providerSecrets and also ensures that legacy API key fields are updated for backward compatibility. Furthermore, it propagates API key changes to customApiModes and the apiMode if they are currently selected, ensuring consistency across the application. This function is vital for the new unified secret management system.

src/popup/sections/GeneralPart.jsx (197-203)

medium

These new state variables and derived values (selectedProviderRequest, selectedProviderId, selectedProvider, selectedProviderApiKey, isUsingOpenAICompatibleProvider) are crucial for dynamically displaying and managing API keys for the currently selected OpenAI-compatible provider. They leverage the new provider-registry.mjs to resolve provider information.

src/popup/sections/GeneralPart.jsx (206-217)

medium

The getBalance function has been updated to use the selectedProviderApiKey and openAiApiUrl derived from the new provider registry. This ensures that balance checks are performed against the correct API key and endpoint for the currently active OpenAI-compatible provider, centralizing the logic and making it more robust.

src/popup/sections/GeneralPart.jsx (275)

medium

The condition isUsingOpenAICompatibleProvider replaces multiple individual checks for OpenAI-compatible models. This simplifies the conditional rendering logic for the API key input field, making it more concise and easier to understand.

src/popup/sections/GeneralPart.jsx (326-356)

medium

This block now dynamically renders the API key input and balance check button based on whether an isUsingOpenAICompatibleProvider is selected. The selectedProviderApiKey and selectedProviderId are used to manage the input value and conditional rendering of the 'Get'/'Balance' buttons, centralizing API key management.

src/services/apis/custom-api.mjs (1)

medium

The import statement has been simplified to only include generateAnswersWithOpenAICompatible. This is a direct consequence of unifying the OpenAI-compatible API execution into a shared core module, reducing the number of individual API service imports.

src/services/apis/custom-api.mjs (19-29)

medium

The generateAnswersWithCustomApi function has been refactored to delegate its logic to the new generateAnswersWithOpenAICompatible function. This significantly reduces code duplication and centralizes the core logic for handling OpenAI-compatible API requests, including custom ones. The allowLegacyResponseField: true is important for maintaining backward compatibility with custom APIs that might use a response field.

src/services/apis/openai-api.mjs (3-4)

medium

The import of generateAnswersWithOpenAICompatible and resolveOpenAICompatibleRequest from the new core and provider registry modules is key to unifying API handling. This allows openai-api.mjs to leverage the shared logic instead of duplicating it.

src/services/apis/openai-api.mjs (6-8)

medium

The normalizeBaseUrl function is a useful utility to ensure consistency in API endpoint URLs by trimming trailing slashes. This helps prevent issues with URL construction and improves the robustness of API requests.

src/services/apis/openai-api.mjs (10-21)

medium

The resolveModelName function centralizes the logic for determining the actual model name to be used in API requests, especially for custom models and API modes. This ensures that the correct model identifier is passed to the unified API handler.

src/services/apis/openai-api.mjs (24-40)

medium

The touchOllamaKeepAlive function is a new addition specifically for Ollama providers. It sends a small request to the Ollama endpoint to keep the model loaded, which is important for performance and responsiveness. This demonstrates how provider-specific logic can be integrated while still using a unified API flow.

src/services/apis/openai-api.mjs (48-58)

medium

The generateAnswersWithGptCompletionApi function has been refactored to use the new generateAnswersWithOpenAICompatible core function. This significantly reduces code duplication and centralizes the logic for handling completion-type API requests, improving maintainability.

src/services/apis/openai-api.mjs (69-70)

medium

The baseUrl for generateAnswersWithChatgptApiCompat now uses normalizeBaseUrl, ensuring consistent URL formatting. This is a minor but good improvement for robustness.

src/services/apis/openai-api.mjs (89-99)

medium

The generateAnswersWithChatgptApiCompat function has been refactored to use the new generateAnswersWithOpenAICompatible core function. This change eliminates duplicated SSE fetching logic and centralizes the handling of chat-completion type API requests, making the code cleaner and more maintainable.

src/services/apis/openai-api.mjs (109-133)

medium

This new function, generateAnswersWithOpenAICompatibleApi, serves as the unified entry point for all OpenAI-compatible providers. It resolves the correct provider, endpoint type, URL, API key, and model using the new provider registry, then delegates to generateAnswersWithOpenAICompatible. This is the cornerstone of the PR's unification efforts, drastically simplifying the executeApi logic in background/index.mjs.

src/services/apis/openai-compatible-core.mjs (8-14)

medium

The buildHeaders function centralizes the logic for constructing HTTP headers, including the Authorization header with the API key. This promotes consistency and reusability across all OpenAI-compatible API requests.

src/services/apis/openai-compatible-core.mjs (17-28)

medium

The buildMessageAnswer function encapsulates the logic for extracting the answer content from different API response formats (delta, content, text, and legacy response field). This abstraction makes the SSE onMessage handler cleaner and more adaptable to various provider responses.

src/services/apis/openai-compatible-core.mjs (31-33)

medium

The hasFinished function provides a clear and concise way to determine if an API response indicates the end of a stream, based on the finish_reason field. This improves readability in the SSE onMessage handler.

src/services/apis/openai-compatible-core.mjs (49-159)

medium

This new generateAnswersWithOpenAICompatible function is the core of the unified API handling. It abstracts away the details of constructing request bodies for both 'completion' and 'chat' endpoints, handling token parameters, and managing the SSE connection. This significantly reduces code duplication across different OpenAI-compatible API services and makes it easier to add new providers in the future.

src/services/apis/provider-registry.mjs (4-13)

medium

The LEGACY_KEY_BY_PROVIDER_ID constant is crucial for the migration process, mapping older configuration keys to the new provider IDs. This ensures that existing user configurations are correctly interpreted and migrated to the unified secret management system.

src/services/apis/provider-registry.mjs (15-85)

medium

The BUILTIN_PROVIDER_TEMPLATE defines a standardized structure for all built-in OpenAI-compatible providers. This template includes essential information like ID, name, base URLs, and paths, which is fundamental for the new provider registry and unified API handling. It also includes allowLegacyResponseField for backward compatibility.

src/services/apis/provider-registry.mjs (88-98)

medium

The OPENAI_COMPATIBLE_GROUP_TO_PROVIDER_ID mapping is vital for translating legacy API mode group names into the new standardized provider IDs. This enables the system to correctly identify which provider corresponds to a given API mode, facilitating the migration and unification.

src/services/apis/provider-registry.mjs (101-134)

medium

The getModelNamePresetPart and resolveProviderIdFromLegacyModelName functions are essential for backward compatibility. They allow the system to infer the correct provider ID from older, less structured model names, ensuring that existing user configurations continue to work seamlessly with the new provider registry.

src/services/apis/provider-registry.mjs (136-139)

medium

The isLegacyCompletionModelName function helps identify models that historically used the 'completion' endpoint. This is important for correctly routing requests to the appropriate endpoint type within the unified API handler.

src/services/apis/provider-registry.mjs (141-157)

medium

The toStringOrEmpty, trimSlashes, ensureLeadingSlash, and joinUrl utility functions provide robust and consistent string manipulation for URLs and paths. These are crucial for correctly constructing API endpoints, especially when dealing with user-provided custom URLs and paths.

src/services/apis/provider-registry.mjs (160-184)

medium

The buildBuiltinProviders function dynamically constructs the list of built-in providers, incorporating user-specific configuration values like customOpenAiApiUrl and ollamaEndpoint. This ensures that built-in providers are correctly configured based on user settings.

src/services/apis/provider-registry.mjs (186-201)

medium

The normalizeCustomProvider function ensures that custom provider configurations are consistently structured and validated. It assigns default values and normalizes fields like id, name, baseUrl, and paths, which is essential for reliable custom provider management.

src/services/apis/provider-registry.mjs (203-208)

medium

The getCustomOpenAIProviders function retrieves and normalizes custom provider configurations from the user's settings. This provides a clean, consistent list of custom providers for use throughout the application.

src/services/apis/provider-registry.mjs (210-212)

medium

The getAllOpenAIProviders function combines both built-in and custom providers into a single, comprehensive list. This unified list is then used by other functions to resolve provider details.

src/services/apis/provider-registry.mjs (215-228)

medium

The resolveProviderIdForSession function is crucial for determining the correct provider ID based on the current session's API mode or model name. It handles both new API mode structures and legacy model names, ensuring that the appropriate provider is identified for each request.

src/services/apis/provider-registry.mjs (230-238)

medium

The resolveEndpointTypeForSession function determines whether a session should use a 'chat' or 'completion' endpoint. This is important for correctly formatting the request body in the unified API handler, especially for legacy completion models.

src/services/apis/provider-registry.mjs (240-246)

medium

The getProviderById function retrieves a provider's full configuration by its ID. It filters out disabled providers, ensuring that only active and valid providers are used.

src/services/apis/provider-registry.mjs (248-268)

medium

The getProviderSecret function centralizes the logic for retrieving API keys. It prioritizes API keys specified directly in the apiMode, then checks the new providerSecrets map, and finally falls back to legacy API key fields. This ensures that the correct API key is used for each provider, supporting both new and old configurations.

src/services/apis/provider-registry.mjs (270-301)

medium

The resolveUrlFromProvider function constructs the full API request URL based on the provider's configuration and the endpoint type (chat or completion). It handles custom URLs specified in the API mode, as well as base URLs and paths defined in the provider template, ensuring accurate endpoint resolution.

src/services/apis/provider-registry.mjs (303-317)

medium

The resolveOpenAICompatibleRequest function is the main entry point for obtaining all necessary information to make an OpenAI-compatible API request. It orchestrates the resolution of provider ID, provider details, endpoint type, request URL, and API key, providing a complete request object to the unified API handler.

src/services/init-session.mjs (5)

medium

The import of normalizeApiMode is essential for ensuring that API mode objects are consistently structured and validated when a new session is initialized. This helps prevent unexpected behavior due to malformed API mode data.

src/services/init-session.mjs (75)

medium

The apiMode property is now explicitly normalized using normalizeApiMode during session initialization. This ensures that all API mode objects conform to a consistent structure, which is critical for the new provider registry and unified API handling.

src/services/wrappers.mjs (13)

medium

The import of normalizeApiMode is crucial here to ensure that any API mode object received or processed by the port listener is normalized. This guarantees consistency and prevents issues with potentially malformed API mode data.

src/services/wrappers.mjs (110)

medium

The session.apiMode is now explicitly normalized using normalizeApiMode within the port listener. This ensures that any API mode passed into the executeApi function is in a consistent and expected format, which is vital for the new unified API handling logic.

src/utils/model-name-convert.mjs (81-93)

medium

The new normalizeApiMode function ensures that API mode objects always have a consistent structure and default values. This is critical for reliable processing of API modes throughout the application, especially with the introduction of new fields like providerId and the migration logic.

src/utils/model-name-convert.mjs (97-98)

medium

The apiModeToModelName function now explicitly normalizes the apiMode object at the beginning. This ensures that any subsequent logic in the function operates on a well-formed API mode object, improving robustness.

src/utils/model-name-convert.mjs (111-115)

medium

The getApiModesFromConfig function now normalizes config.customApiModes before processing them. This ensures that custom API modes are always in a consistent format, which is important for accurate filtering and mapping.

src/utils/model-name-convert.mjs (132-133)

medium

The logic for Ollama model names has been updated to explicitly set modelName to ollamaModel- followed by the configured Ollama model name. This ensures consistency in how Ollama models are identified within the system, aligning with the new provider registry.

src/utils/model-name-convert.mjs (139)

medium

The normalizedCustomApiModes are now correctly filtered and included in the returned array. This ensures that all custom API modes, after normalization, are properly accounted for when retrieving the list of available API modes.

src/utils/model-name-convert.mjs (148-157)

medium

The normalizeForCompare helper function is introduced to create a consistent string representation of API mode objects for comparison. This is crucial for accurately determining if an API mode is selected, especially with the addition of new fields like providerId and the need to ignore transient fields like apiKey and customUrl during comparison.

src/utils/model-name-convert.mjs (160)

medium

The isApiModeSelected function now uses the normalizeForCompare helper to compare API mode objects. This ensures that comparisons are robust and accurate, preventing false negatives or positives due to differences in non-essential fields or object ordering.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (4)
src/popup/sections/GeneralPart.jsx (3)

197-203: Redundant provider lookup — resolveOpenAICompatibleRequest already returns provider.

Per provider-registry.mjs (lines 302-317), resolveOpenAICompatibleRequest returns { providerId, provider, endpointType, requestUrl, apiKey }. The getProviderById call on line 200 duplicates the lookup that was already performed.

♻️ Suggested simplification
   const selectedProviderRequest = resolveOpenAICompatibleRequest(config, config)
   const selectedProviderId = selectedProviderRequest?.providerId || ''
-  const selectedProvider = selectedProviderRequest
-    ? getProviderById(config, selectedProviderRequest.providerId)
-    : null
+  const selectedProvider = selectedProviderRequest?.provider || null
   const selectedProviderApiKey = selectedProviderRequest?.apiKey || ''
   const isUsingOpenAICompatibleProvider = Boolean(selectedProviderRequest)

This also means getProviderById can be removed from the imports on line 29 if unused elsewhere in this file.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/GeneralPart.jsx` around lines 197 - 203, The code performs
a redundant provider lookup: resolveOpenAICompatibleRequest already returns a
provider object, so remove the extra getProviderById call and use the provider
from selectedProviderRequest directly. Replace usage of selectedProvider
(currently assigned via getProviderById) with selectedProviderRequest.provider,
keep selectedProviderRequest, selectedProviderApiKey and
isUsingOpenAICompatibleProvider as-is, and remove getProviderById from imports
if no other references remain in this file.

205-219: Billing URL construction assumes baseUrl does not include a path prefix.

Line 207 concatenates openAiApiUrl + '/dashboard/billing/credit_grants', and checkBilling (line 54) uses apiUrl + '/v1/dashboard/billing/...'. This works correctly only when baseUrl is a bare origin like https://api.openai.com.

Since the Get/Balance buttons are gated to selectedProviderId === 'openai' (line 337), this is safe today — but worth a brief comment in case the billing check is ever extended to other providers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/GeneralPart.jsx` around lines 205 - 219, getBalance
constructs billing paths by simple string concatenation using
selectedProvider?.baseUrl or config.customOpenAiApiUrl which breaks if baseUrl
contains a path prefix; change getBalance to build the billing URL with a robust
approach (e.g., use the URL constructor or ensure path joining) instead of naive
concatenation, mirroring how checkBilling constructs URLs (apiUrl + '/v1/...');
update the code in getBalance (and note behavior in a brief comment near
getBalance and checkBilling) to use new URL('/dashboard/billing/credit_grants',
openAiApiUrl) or equivalent so it works when baseUrl includes a path, and keep
existing fallbacks (checkBilling and openUrl) unchanged.

107-181: Consider extracting buildProviderSecretUpdate to a shared utility module.

This ~75-line function contains non-trivial business logic for provider secret syncing, legacy key migration, and API mode key reconciliation. Placing it alongside the UI component makes this file harder to test in isolation and harder to reuse if other entry points need the same logic.

A module like src/services/apis/provider-secrets.mjs (or similar) would be a better home. As per coding guidelines, src/services/apis/**/*.{js,mjs}: "API client integrations should be placed in src/services/apis/" — while this isn't strictly an API client, it's tightly coupled to the provider registry's data model.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/GeneralPart.jsx` around lines 107 - 181, The function
buildProviderSecretUpdate contains complex provider-secret sync and migration
logic and should be moved out of the UI component into a shared service module
for testability and reuse; extract buildProviderSecretUpdate (and any helper
symbols it relies on like LEGACY_API_KEY_FIELD_BY_PROVIDER_ID and
isApiModeSelected) into a new module under src/services/apis (e.g.,
src/services/apis/provider-secrets.mjs), update the GeneralPart.jsx import to
use the new exported function, adjust exports so unit tests can import the logic
directly, and ensure any references to config shape remain unchanged so behavior
stays identical.
src/popup/sections/ApiModes.jsx (1)

42-70: normalizeBaseUrl is duplicated in openai-api.mjs.

The identical helper (strip trailing slashes from a URL string) exists at src/services/apis/openai-api.mjs lines 6-8. Consider extracting it to a shared utility to avoid drift.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/ApiModes.jsx` around lines 42 - 70, The helper
normalizeBaseUrl is duplicated (also present in openai-api.mjs); extract it into
a shared utility module (e.g., utils/url.js or similar), export
normalizeBaseUrl, replace the local definitions in ApiModes.jsx and
openai-api.mjs with imports of the shared function, update any imports/usages to
the new export name, and remove the duplicate implementations so both
createProviderId/normalizeProviderId callers use the single centralized
normalizeBaseUrl function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/background/index.mjs`:
- Around line 8-9: Update the import for generateAnswersWithOpenAICompatibleApi
to include the explicit .mjs extension (match the style used for
generateAnswersWithAzureOpenaiApi); change the import source
'../services/apis/openai-api' to '../services/apis/openai-api.mjs' so the module
loader can resolve the file at runtime and remain consistent with other imports
(reference symbol: generateAnswersWithOpenAICompatibleApi).

In `@src/popup/sections/ApiModes.jsx`:
- Around line 144-147: The save handler for new providers silently returns when
providerName or providerBaseUrl is empty (code around providerSelector ===
'__new__', providerDraft, normalizeBaseUrl); update the component to perform
explicit validation: compute a boolean like isNewProviderValid =
Boolean(providerDraft.name.trim() && normalizeBaseUrl(providerDraft.baseUrl))
and use it to disable the Save button and/or set an inline validation message
state (e.g., providerError) shown near the name/baseUrl inputs; also ensure the
save handler shows that validation message when invoked with invalid input
instead of simply returning.
- Around line 337-348: When opening the editor for an apiMode in ApiModes.jsx,
validate the resolved providerId (computed in the isCustomApiMode branch)
against the current customProviders list and if it no longer exists, replace it
with LEGACY_CUSTOM_PROVIDER_ID before calling setEditingApiMode and
setProviderSelector; specifically, update the logic around
isCustomApiMode/providerId so you check customProviders for a matching
providerId and fall back to LEGACY_CUSTOM_PROVIDER_ID, then call
setEditingApiMode({...defaultApiMode, ...apiMode, providerId:
validatedProviderId}) and setProviderSelector(validatedProviderId) to avoid
persisting a stale ID on save.

In `@src/popup/sections/GeneralPart.jsx`:
- Around line 96-105: Extract the canonical provider→field mapping currently
defined as LEGACY_API_KEY_FIELD_BY_PROVIDER_ID and the inverse
LEGACY_SECRET_KEY_TO_PROVIDER_ID into a single shared constants module, export
the provider→field map (e.g., LEGACY_API_KEY_FIELD_BY_PROVIDER_ID) from that
module, and replace local copies in GeneralPart.jsx and the config module by
importing that constant; then derive the inverse mapping programmatically (e.g.,
by reversing the imported provider→field map into
LEGACY_SECRET_KEY_TO_PROVIDER_ID where needed) so future provider additions only
require updating one source of truth.

In `@src/services/apis/openai-api.mjs`:
- Around line 24-40: The touchOllamaKeepAlive function issues a fetch without a
timeout which can hang; update touchOllamaKeepAlive to create an
AbortController, pass controller.signal into fetch, start a timer (e.g.,
setTimeout) to call controller.abort() after a short configurable timeout
(milliseconds), ensure the timer is cleared on success/failure, and handle the
abort error so the caller (e.g., generateAnswersWithOpenAICompatibleApi) won't
wait indefinitely; keep the same request shape and headers but include the
signal and proper cleanup.

In `@src/services/apis/provider-registry.mjs`:
- Around line 160-183: The openai provider base URL can end with "/v1" and cause
duplicate "/v1" when later joined; in buildBuiltinProviders normalize
config.customOpenAiApiUrl by stripping any trailing "/v1" or "/v1/" before
calling trimSlashes (i.e., compute a normalized variable from
config.customOpenAiApiUrl that removes a trailing "/v1" segment if present, then
pass that into trimSlashes for the openai branch); reference
buildBuiltinProviders, config.customOpenAiApiUrl, and trimSlashes when making
this change.
- Around line 270-280: resolveUrlFromProvider currently unconditionally prefers
session.apiMode.customUrl for customApiModelKeys, which causes any provider
config (including provider.providerId) to be ignored; update the logic in
resolveUrlFromProvider to only use session.apiMode.customUrl when there is no
provider.providerId (or when an explicit override flag like
session.apiMode.forceCustomUrl is truthy), otherwise fall back to reading the
provider's configured URL from the provider/config arguments; reference
resolveUrlFromProvider, session.apiMode.customUrl, and provider.providerId when
making the change.

---

Nitpick comments:
In `@src/popup/sections/ApiModes.jsx`:
- Around line 42-70: The helper normalizeBaseUrl is duplicated (also present in
openai-api.mjs); extract it into a shared utility module (e.g., utils/url.js or
similar), export normalizeBaseUrl, replace the local definitions in ApiModes.jsx
and openai-api.mjs with imports of the shared function, update any
imports/usages to the new export name, and remove the duplicate implementations
so both createProviderId/normalizeProviderId callers use the single centralized
normalizeBaseUrl function.

In `@src/popup/sections/GeneralPart.jsx`:
- Around line 197-203: The code performs a redundant provider lookup:
resolveOpenAICompatibleRequest already returns a provider object, so remove the
extra getProviderById call and use the provider from selectedProviderRequest
directly. Replace usage of selectedProvider (currently assigned via
getProviderById) with selectedProviderRequest.provider, keep
selectedProviderRequest, selectedProviderApiKey and
isUsingOpenAICompatibleProvider as-is, and remove getProviderById from imports
if no other references remain in this file.
- Around line 205-219: getBalance constructs billing paths by simple string
concatenation using selectedProvider?.baseUrl or config.customOpenAiApiUrl which
breaks if baseUrl contains a path prefix; change getBalance to build the billing
URL with a robust approach (e.g., use the URL constructor or ensure path
joining) instead of naive concatenation, mirroring how checkBilling constructs
URLs (apiUrl + '/v1/...'); update the code in getBalance (and note behavior in a
brief comment near getBalance and checkBilling) to use new
URL('/dashboard/billing/credit_grants', openAiApiUrl) or equivalent so it works
when baseUrl includes a path, and keep existing fallbacks (checkBilling and
openUrl) unchanged.
- Around line 107-181: The function buildProviderSecretUpdate contains complex
provider-secret sync and migration logic and should be moved out of the UI
component into a shared service module for testability and reuse; extract
buildProviderSecretUpdate (and any helper symbols it relies on like
LEGACY_API_KEY_FIELD_BY_PROVIDER_ID and isApiModeSelected) into a new module
under src/services/apis (e.g., src/services/apis/provider-secrets.mjs), update
the GeneralPart.jsx import to use the new exported function, adjust exports so
unit tests can import the logic directly, and ensure any references to config
shape remain unchanged so behavior stays identical.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab628e1 and 36ed81e.

📒 Files selected for processing (11)
  • src/background/index.mjs
  • src/config/index.mjs
  • src/popup/sections/ApiModes.jsx
  • src/popup/sections/GeneralPart.jsx
  • src/services/apis/custom-api.mjs
  • src/services/apis/openai-api.mjs
  • src/services/apis/openai-compatible-core.mjs
  • src/services/apis/provider-registry.mjs
  • src/services/init-session.mjs
  • src/services/wrappers.mjs
  • src/utils/model-name-convert.mjs

Comment on lines +8 to 9
import { generateAnswersWithOpenAICompatibleApi } from '../services/apis/openai-api'
import { generateAnswersWithAzureOpenaiApi } from '../services/apis/azure-openai-api.mjs'
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, check what files exist in the openai-api path
echo "=== Checking openai-api related files ==="
fd -a 'openai-api' src/services/apis 2>/dev/null || echo "No matches with exact name"
fd 'openai.*api' src/services/apis 2>/dev/null || echo "No matches with pattern"

echo ""
echo "=== Checking all import statements for openai-api in src ==="
rg -n "openai-api" src 2>/dev/null || echo "No matches found"

echo ""
echo "=== Checking bundler/build config files ==="
fd -a 'vite.config.*|webpack.config.*|rollup.config.*|esbuild.*|parcel.*' . 2>/dev/null | head -20

echo ""
echo "=== Checking package.json for module resolution settings ==="
if [ -f "package.json" ]; then
  cat package.json | jq '.type // .exports // .main // empty' 2>/dev/null || cat package.json | grep -A5 '"type"\|"exports"\|"main"' 2>/dev/null || echo "No explicit module resolution found"
fi

echo ""
echo "=== Listing files in src/services/apis directory ==="
ls -la src/services/apis/ 2>/dev/null || echo "Directory not found"

Repository: ChatGPTBox-dev/chatGPTBox

Length of output: 2909


Add .mjs extension to openai-api import for consistency.

The file src/services/apis/openai-api.mjs exists, and every other import of it throughout the codebase uses the explicit .mjs extension (e.g., in ollama-api.mjs, openrouter-api.mjs). Line 9 of the same file already correctly imports azure-openai-api.mjs with the extension. Without explicit file extensions, the import will fail at runtime.

Fix
-import { generateAnswersWithOpenAICompatibleApi } from '../services/apis/openai-api'
+import { generateAnswersWithOpenAICompatibleApi } from '../services/apis/openai-api.mjs'
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { generateAnswersWithOpenAICompatibleApi } from '../services/apis/openai-api'
import { generateAnswersWithAzureOpenaiApi } from '../services/apis/azure-openai-api.mjs'
import { generateAnswersWithOpenAICompatibleApi } from '../services/apis/openai-api.mjs'
import { generateAnswersWithAzureOpenaiApi } from '../services/apis/azure-openai-api.mjs'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/background/index.mjs` around lines 8 - 9, Update the import for
generateAnswersWithOpenAICompatibleApi to include the explicit .mjs extension
(match the style used for generateAnswersWithAzureOpenaiApi); change the import
source '../services/apis/openai-api' to '../services/apis/openai-api.mjs' so the
module loader can resolve the file at runtime and remain consistent with other
imports (reference symbol: generateAnswersWithOpenAICompatibleApi).

Comment on lines +144 to +147
if (providerSelector === '__new__') {
const providerName = providerDraft.name.trim()
const providerBaseUrl = normalizeBaseUrl(providerDraft.baseUrl)
if (!providerName || !providerBaseUrl) return
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Silent validation failure when creating a new provider.

When providerName or providerBaseUrl is empty, the save silently aborts (line 147). The user gets no indication that their input was invalid. Consider disabling the Save button or displaying an inline validation message.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/ApiModes.jsx` around lines 144 - 147, The save handler for
new providers silently returns when providerName or providerBaseUrl is empty
(code around providerSelector === '__new__', providerDraft, normalizeBaseUrl);
update the component to perform explicit validation: compute a boolean like
isNewProviderValid = Boolean(providerDraft.name.trim() &&
normalizeBaseUrl(providerDraft.baseUrl)) and use it to disable the Save button
and/or set an inline validation message state (e.g., providerError) shown near
the name/baseUrl inputs; also ensure the save handler shows that validation
message when invoked with invalid input instead of simply returning.

Comment on lines +337 to 348
const isCustomApiMode = apiMode.groupName === 'customApiModelKeys'
const providerId = isCustomApiMode
? apiMode.providerId || LEGACY_CUSTOM_PROVIDER_ID
: ''
setEditingApiMode({
...defaultApiMode,
...apiMode,
providerId,
})
setProviderSelector(providerId || LEGACY_CUSTOM_PROVIDER_ID)
setProviderDraft(defaultProviderDraft)
setEditingIndex(index)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Edge case: stale providerId if the referenced provider was removed from config.

When editing an apiMode whose providerId no longer exists in customProviders, the providerSelector state will hold a value that matches no <select> option. The <select> would visually show the first option ("Custom") but the backing state remains stale. On save, the stale ID would be persisted via line 182.

Since provider deletion doesn't appear to be implemented yet, this is low-risk. Worth a note for when that feature is added — the edit handler should validate providerId against the current provider list.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/ApiModes.jsx` around lines 337 - 348, When opening the
editor for an apiMode in ApiModes.jsx, validate the resolved providerId
(computed in the isCustomApiMode branch) against the current customProviders
list and if it no longer exists, replace it with LEGACY_CUSTOM_PROVIDER_ID
before calling setEditingApiMode and setProviderSelector; specifically, update
the logic around isCustomApiMode/providerId so you check customProviders for a
matching providerId and fall back to LEGACY_CUSTOM_PROVIDER_ID, then call
setEditingApiMode({...defaultApiMode, ...apiMode, providerId:
validatedProviderId}) and setProviderSelector(validatedProviderId) to avoid
persisting a stale ID on save.

Comment on lines +96 to +105
const LEGACY_API_KEY_FIELD_BY_PROVIDER_ID = {
openai: 'apiKey',
deepseek: 'deepSeekApiKey',
moonshot: 'moonshotApiKey',
openrouter: 'openRouterApiKey',
aiml: 'aimlApiKey',
chatglm: 'chatglmApiKey',
ollama: 'ollamaApiKey',
'legacy-custom-default': 'customApiKey',
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for LEGACY_API_KEY_FIELD constant
echo "=== Searching for LEGACY_API_KEY_FIELD ==="
rg -n "LEGACY_API_KEY_FIELD" --type=js --type=jsx

echo ""
echo "=== Searching for provider-to-key mappings ==="
rg -nC3 "deepSeekApiKey|moonshotApiKey|openRouterApiKey|aimlApiKey|chatglmApiKey|ollamaApiKey" --type=js --type=jsx | head -100

echo ""
echo "=== Checking src/config/index.mjs for similar structures ==="
if [ -f "src/config/index.mjs" ]; then
  cat -n src/config/index.mjs | grep -i "legacy\|deepseek\|moonshot\|openrouter" || echo "No matches in config"
fi

echo ""
echo "=== Checking provider-registry for mappings ==="
if [ -f "src/common/provider-registry.mjs" ]; then
  cat -n src/common/provider-registry.mjs | grep -i "legacy\|deepseek\|moonshot\|api.*key" || echo "No legacy API key mappings found"
fi

echo ""
echo "=== List config-related files ==="
fd "config|provider" --type=f src/

Repository: ChatGPTBox-dev/chatGPTBox

Length of output: 7227


🏁 Script executed:

#!/bin/bash
# Find GeneralPart.jsx
echo "=== Finding GeneralPart.jsx ==="
fd "GeneralPart" --type=f

echo ""
echo "=== Reading GeneralPart.jsx lines 90-110 ==="
if [ -f "src/popup/sections/GeneralPart.jsx" ]; then
  sed -n '90,110p' src/popup/sections/GeneralPart.jsx
else
  echo "File not found at expected path"
fi

echo ""
echo "=== Comparing with LEGACY_SECRET_KEY_TO_PROVIDER_ID in config/index.mjs ==="
echo "From src/config/index.mjs (lines 730-738):"
sed -n '730,738p' src/config/index.mjs

echo ""
echo "=== Search for uses of LEGACY_API_KEY_FIELD_BY_PROVIDER_ID ==="
rg "LEGACY_API_KEY_FIELD_BY_PROVIDER_ID" --no-ignore

echo ""
echo "=== Search for uses of LEGACY_SECRET_KEY_TO_PROVIDER_ID ==="
rg "LEGACY_SECRET_KEY_TO_PROVIDER_ID" --no-ignore

Repository: ChatGPTBox-dev/chatGPTBox

Length of output: 1701


Extract LEGACY_API_KEY_FIELD_BY_PROVIDER_ID to a shared location to avoid duplication.

This mapping is duplicated in src/config/index.mjs as LEGACY_SECRET_KEY_TO_PROVIDER_ID (lines 730-738). Both constants contain identical data in inverse directions (provider_id → key field name vs. key field name → provider_id). If a new provider with legacy API key support is added, both must be updated, creating a maintenance risk. Move the canonical mapping to a shared constants file and derive the inverse mapping programmatically where needed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/popup/sections/GeneralPart.jsx` around lines 96 - 105, Extract the
canonical provider→field mapping currently defined as
LEGACY_API_KEY_FIELD_BY_PROVIDER_ID and the inverse
LEGACY_SECRET_KEY_TO_PROVIDER_ID into a single shared constants module, export
the provider→field map (e.g., LEGACY_API_KEY_FIELD_BY_PROVIDER_ID) from that
module, and replace local copies in GeneralPart.jsx and the config module by
importing that constant; then derive the inverse mapping programmatically (e.g.,
by reversing the imported provider→field map into
LEGACY_SECRET_KEY_TO_PROVIDER_ID where needed) so future provider additions only
require updating one source of truth.

Comment on lines +24 to +40
async function touchOllamaKeepAlive(config, model, apiKey) {
return fetch(`${normalizeBaseUrl(config.ollamaEndpoint)}/api/generate`, {
method: 'POST',
signal: controller.signal,
headers: {
'Content-Type': 'application/json',
Authorization: `Bearer ${apiKey}`,
...(apiKey ? { Authorization: `Bearer ${apiKey}` } : {}),
},
body: JSON.stringify({
prompt: prompt,
model,
stream: true,
max_tokens: config.maxResponseTokenLength,
temperature: config.temperature,
stop: '\nHuman',
prompt: 't',
options: {
num_predict: 1,
},
keep_alive: config.ollamaKeepAliveTime === '-1' ? -1 : config.ollamaKeepAliveTime,
}),
onMessage(message) {
console.debug('sse message', message)
if (finished) return
if (message.trim() === '[DONE]') {
finish()
return
}
let data
try {
data = JSON.parse(message)
} catch (error) {
console.debug('json error', error)
return
}

answer += data.choices[0].text
port.postMessage({ answer: answer, done: false, session: null })
})
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing timeout on touchOllamaKeepAlive fetch — can block indefinitely.

The keep-alive fetch has no AbortController timeout. If the Ollama endpoint is unresponsive, this await (line 129) will hang the enclosing function indefinitely. While the main streaming response is already complete at this point, the calling code may still be waiting for the generateAnswersWithOpenAICompatibleApi promise to settle.

🛡️ Suggested fix: add a timeout
 async function touchOllamaKeepAlive(config, model, apiKey) {
+  const controller = new AbortController()
+  const timeout = setTimeout(() => controller.abort(), 5000)
   return fetch(`${normalizeBaseUrl(config.ollamaEndpoint)}/api/generate`, {
     method: 'POST',
+    signal: controller.signal,
     headers: {
       'Content-Type': 'application/json',
       ...(apiKey ? { Authorization: `Bearer ${apiKey}` } : {}),
     },
     body: JSON.stringify({
       model,
       prompt: 't',
       options: {
         num_predict: 1,
       },
       keep_alive: config.ollamaKeepAliveTime === '-1' ? -1 : config.ollamaKeepAliveTime,
     }),
-  })
+  }).finally(() => clearTimeout(timeout))
 }

Also applies to: 128-132

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/apis/openai-api.mjs` around lines 24 - 40, The
touchOllamaKeepAlive function issues a fetch without a timeout which can hang;
update touchOllamaKeepAlive to create an AbortController, pass controller.signal
into fetch, start a timer (e.g., setTimeout) to call controller.abort() after a
short configurable timeout (milliseconds), ensure the timer is cleared on
success/failure, and handle the abort error so the caller (e.g.,
generateAnswersWithOpenAICompatibleApi) won't wait indefinitely; keep the same
request shape and headers but include the signal and proper cleanup.

Comment on lines +160 to +183
function buildBuiltinProviders(config) {
return BUILTIN_PROVIDER_TEMPLATE.map((provider) => {
if (provider.id === 'openai') {
return {
...provider,
baseUrl: trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com'),
}
}
if (provider.id === 'ollama') {
return {
...provider,
baseUrl: `${trimSlashes(config.ollamaEndpoint || 'http://127.0.0.1:11434')}/v1`,
}
}
if (provider.id === 'legacy-custom-default') {
return {
...provider,
chatCompletionsUrl:
toStringOrEmpty(config.customModelApiUrl).trim() ||
'http://localhost:8000/v1/chat/completions',
}
}
return provider
})
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against /v1 duplication in customOpenAiApiUrl.

If users previously stored a base URL ending in /v1, joinUrl will produce /v1/v1/... and break requests. Consider normalizing the base URL to strip a trailing /v1 before appending paths.

🛠️ Proposed fix
-    if (provider.id === 'openai') {
-      return {
-        ...provider,
-        baseUrl: trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com'),
-      }
-    }
+    if (provider.id === 'openai') {
+      const rawBaseUrl = trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com')
+      const normalizedBaseUrl = rawBaseUrl.replace(/\/v1$/, '')
+      return {
+        ...provider,
+        baseUrl: normalizedBaseUrl,
+      }
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function buildBuiltinProviders(config) {
return BUILTIN_PROVIDER_TEMPLATE.map((provider) => {
if (provider.id === 'openai') {
return {
...provider,
baseUrl: trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com'),
}
}
if (provider.id === 'ollama') {
return {
...provider,
baseUrl: `${trimSlashes(config.ollamaEndpoint || 'http://127.0.0.1:11434')}/v1`,
}
}
if (provider.id === 'legacy-custom-default') {
return {
...provider,
chatCompletionsUrl:
toStringOrEmpty(config.customModelApiUrl).trim() ||
'http://localhost:8000/v1/chat/completions',
}
}
return provider
})
function buildBuiltinProviders(config) {
return BUILTIN_PROVIDER_TEMPLATE.map((provider) => {
if (provider.id === 'openai') {
const rawBaseUrl = trimSlashes(config.customOpenAiApiUrl || 'https://api.openai.com')
const normalizedBaseUrl = rawBaseUrl.replace(/\/v1$/, '')
return {
...provider,
baseUrl: normalizedBaseUrl,
}
}
if (provider.id === 'ollama') {
return {
...provider,
baseUrl: `${trimSlashes(config.ollamaEndpoint || 'http://127.0.0.1:11434')}/v1`,
}
}
if (provider.id === 'legacy-custom-default') {
return {
...provider,
chatCompletionsUrl:
toStringOrEmpty(config.customModelApiUrl).trim() ||
'http://localhost:8000/v1/chat/completions',
}
}
return provider
})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/apis/provider-registry.mjs` around lines 160 - 183, The openai
provider base URL can end with "/v1" and cause duplicate "/v1" when later
joined; in buildBuiltinProviders normalize config.customOpenAiApiUrl by
stripping any trailing "/v1" or "/v1/" before calling trimSlashes (i.e., compute
a normalized variable from config.customOpenAiApiUrl that removes a trailing
"/v1" segment if present, then pass that into trimSlashes for the openai
branch); reference buildBuiltinProviders, config.customOpenAiApiUrl, and
trimSlashes when making this change.

Comment on lines +270 to +280
function resolveUrlFromProvider(provider, endpointType, config, session) {
if (!provider) return ''

const apiModeCustomUrl =
endpointType === 'chat' &&
session?.apiMode &&
typeof session.apiMode === 'object' &&
session.apiMode.groupName === 'customApiModelKeys'
? toStringOrEmpty(session.apiMode.customUrl).trim()
: ''
if (apiModeCustomUrl) return apiModeCustomUrl
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

customUrl currently overrides provider config even when providerId is set.

Because resolveUrlFromProvider always prefers session.apiMode.customUrl for customApiModelKeys, migrated modes will keep using the legacy URL and ignore later provider edits. Consider honoring customUrl only when no providerId is set (or gate it behind an explicit override flag).

🛠️ Proposed fix
-  const apiModeCustomUrl =
-    endpointType === 'chat' &&
-    session?.apiMode &&
-    typeof session.apiMode === 'object' &&
-    session.apiMode.groupName === 'customApiModelKeys'
-      ? toStringOrEmpty(session.apiMode.customUrl).trim()
-      : ''
+  const apiModeCustomUrl =
+    endpointType === 'chat' &&
+    session?.apiMode &&
+    typeof session.apiMode === 'object' &&
+    session.apiMode.groupName === 'customApiModelKeys' &&
+    !toStringOrEmpty(session.apiMode.providerId).trim()
+      ? toStringOrEmpty(session.apiMode.customUrl).trim()
+      : ''
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/apis/provider-registry.mjs` around lines 270 - 280,
resolveUrlFromProvider currently unconditionally prefers
session.apiMode.customUrl for customApiModelKeys, which causes any provider
config (including provider.providerId) to be ignored; update the logic in
resolveUrlFromProvider to only use session.apiMode.customUrl when there is no
provider.providerId (or when an explicit override flag like
session.apiMode.forceCustomUrl is truthy), otherwise fall back to reading the
provider's configured URL from the provider/config arguments; reference
resolveUrlFromProvider, session.apiMode.customUrl, and provider.providerId when
making the change.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant