fix: Format provider API hosts in API server & refactor shared utilities#13198
fix: Format provider API hosts in API server & refactor shared utilities#13198
Conversation
- Move `formatApiHost` and related utilities from renderer to shared package - Create new `@shared/utils/api` module with proper exports - Update renderer to re-export from shared package for backward compatibility - Improve code organization and reuse across codebase
- Move `formatProviderApiHost` from renderer to main process - Apply formatting to providers returned by `/api/providers` endpoint - Ensure consistent API host URLs across all provider types
This reverts commit 27c848f.
Move tests for functions that were refactored into the shared package to their corresponding locations. Add vitest alias config for @shared in the shared test project. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These tests were not part of the original codebase and should not have been added during the migration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mports - Log rejected promises in getAvailableProviders instead of silently dropping providers - Use public entry path for shared utils imports instead of internal module paths - Unify re-export paths to use @shared/aiCore/provider/utils consistently Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note This issue/comment/review was translated by Claude. Self Review Fixes (
|
DeJeune
left a comment
There was a problem hiding this comment.
Overall Assessment
Good PR that addresses a real bug — the API server was using raw provider apiHost values without formatting, causing 404s. The fix in src/main/apiServer/utils/index.ts is clean and well-structured with proper error handling via Promise.allSettled.
The bulk of the diff is a mechanical move of shared utilities from renderer to packages/shared/, which is a sound architectural decision. Tests are comprehensive and CI is green.
Findings
Significant
- Dual
formatProviderApiHostsync risk: The main-process and renderer versions have subtle differences (async vs sync, differentformatVertexApiHostsignatures). The WARNING comment on the renderer side points to the wrong file path (shared/instead ofsrc/main/). Consider adding a bidirectional WARNING so the sync obligation is visible from both files.
Minor
- Variable shadowing (
host) informatVertexApiHost - Loose equality (
==) instead of strict (===) for string comparison - Unrelated
uuidimport change insettings.ts - Surprising test expectations in
formatOllamaApiHostcould use explanatory comments
Positives
- Excellent test coverage for the moved utilities
Promise.allSettledusage in the core fix gracefully handles per-provider formatting failures- Re-exports for backward compatibility is a pragmatic approach
- PR description is thorough with clear explanation of tradeoffs
| projectId: string | ||
| location: string |
There was a problem hiding this comment.
Nit: Variable host on line 10 shadows the function parameter host on line 3. Consider renaming it to baseHost or vertexHost to avoid confusion.
| projectId: string | |
| location: string | |
| const trimmedHost = withoutTrailingSlash(trim(host)) | |
| if (\!trimmedHost || trimmedHost.endsWith('aiplatform.googleapis.com')) { | |
| const baseHost = | |
| location == 'global' ? 'https://aiplatform.googleapis.com' : `https://${location}-aiplatform.googleapis.com` | |
| return `${formatApiHost(baseHost)}/projects/${project}/locations/${location}` |
There was a problem hiding this comment.
renamed to fallbackHost
| OpenAIVerbosity | ||
| } from '@renderer/types/aiCoreTypes' | ||
| import { uuid } from '@renderer/utils' | ||
| import { API_SERVER_DEFAULTS, UpgradeChannel } from '@shared/config/constant' |
There was a problem hiding this comment.
Nit: This import change from @renderer/utils to uuid directly appears unrelated to the PR's scope (API host formatting / shared utility refactor). Was this intentional or a leftover from another change? If intentional, it might be worth mentioning in the PR description.
There was a problem hiding this comment.
Note
This comment was translated by Claude.
Because if we don't make this change, test errors will occur. The uuid imported from renderer/utils is undefined at runtime, likely caused by circular dependencies in the complex dependency chain. Since it's too difficult to troubleshoot, here we directly changed to use the uuid package, and the runtime behavior remains unchanged.
Original Content
因为若不更改就会出现测试错误,从renderer/utils中导入的uuid在运行时为undefined,估计是在复杂的依赖链中出现循环依赖导致的。由于过于难以排查,这里直接改成使用uuid包,运行时行为不变。| import { formatVertexApiHost } from './utils/api' | ||
|
|
||
| /** | ||
| * Format and normalize the API host URL for a provider. | ||
| * Handles provider-specific URL formatting rules (e.g., appending version paths, Azure formatting). | ||
| * | ||
| * @param provider - The provider whose API host is to be formatted. | ||
| * @returns A new provider instance with the formatted API host. | ||
| */ | ||
| export async function formatProviderApiHost(provider: Provider): Promise<Provider> { | ||
| // NOTE: It's async to support Vertex API host formatting | ||
| // Original implementation: src/renderer/src/aiCore/provider/providerConfig.ts |
There was a problem hiding this comment.
Significant: This main-process formatProviderApiHost is a near-duplicate of the renderer version at src/renderer/src/aiCore/provider/providerConfig.ts:formatProviderApiHost. Having two copies that must be manually kept in sync is fragile. A few differences stand out:
- This version is
async(returnsPromise<Provider>) because of Vertex, while the renderer version is sync. - The renderer version uses
formatVertexApiHost(formatted)(passing the full provider), while this version usesformatVertexApiHost(formatted.apiHost)(passing just the string). These are actually different functions with different signatures.
Consider adding a corresponding WARNING comment here pointing back to the renderer version, so the sync obligation is visible from both sides.
Co-authored-by: SuYao <sy20010504@gmail.com>
|
Will resolve conflicts later |
<!-- Template from https://github.com/kubevirt/kubevirt/blob/main/.github/PULL_REQUEST_TEMPLATE.md?--> <!-- Thanks for sending a pull request! Here are some tips for you: 1. Consider creating this PR as draft: https://github.com/CherryHQ/cherry-studio/blob/main/CONTRIBUTING.md --> <!--⚠️ Important: Redux/IndexedDB Data-Changing Feature PRs Temporarily On Hold⚠️ Please note: For our current development cycle, we are not accepting feature Pull Requests that introduce changes to Redux data models or IndexedDB schemas. While we value your contributions, PRs of this nature will be blocked without merge. We welcome all other contributions (bug fixes, perf enhancements, docs, etc.). Thank you! Once version 2.0.0 is released, we will resume reviewing feature PRs. --> ### What this PR does Release Cherry Studio v1.8.2 Before this PR: - Version 1.8.1 is the latest release After this PR: - Version 1.8.2 with bug fixes and model updates <!-- (optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: --> Fixes # ### Why we need it and why it was done in this way This is a patch release that includes: - Bug fixes for knowledge base, message search, and backup restoration - New model support for Xiaomi MiMo-V2-Pro and MiMo-V2-Omni - MiniMax default model upgrade to M2.7 The following tradeoffs were made: - N/A — this is a scheduled patch release The following alternatives were considered: - N/A — standard release workflow Links to places where the discussion took place: <!-- optional: slack, other GH issue, mailinglist, ... --> ### Breaking changes <!-- optional --> None. This is a patch release with no breaking changes. ### Special notes for your reviewer <!-- optional --> **Release Review Checklist:** - [ ] Review generated release notes in `electron-builder.yml` - [ ] Verify version bump in `package.json` (1.8.1 → 1.8.2) - [ ] CI passes - [ ] Merge to trigger release build **Included Commits:** - 8124236 fix(config): update app upgrade segments and include new gateway version - 62c1eb2 fix: correct parameter order in knowledgeSearchTool call (#13635) - bb3dec9 fix(MessageHeader): crash when clicking topic in message search (#13627) - 24645d3 fix(tests): resolve Windows test failures and upgrade prek (#13619) - 0b08fd9 refactor: remove manual install update logic and related API calls - f517a07 fix(BackupManager): update data destination path for backup restoration - f658484 refactor(ConfigManager): remove legacy config migration logic - c1c1b34 chore: Add CI check scripts to package.json (#13564) - 78decc4 feat(model): add support for MiMo-V2-Pro and MiMo-V2-Omni (#13613) - d081b05 fix: Format provider API hosts in API server (#13198) - e4112ba feat: upgrade MiniMax default model to M2.7 (#13593) ### Checklist This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR. Approvers are expected to review this list. - [x] PR: The PR description is expressive enough and will help future contributors - [ ] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [ ] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html) - [ ] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [ ] Self-review: I have reviewed my own code (e.g., via [`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`, or GitHub UI) before requesting review from others ### Release note <!-- Write your release note: 1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required". 2. If no release note is required, just write "NONE". 3. Only include user-facing changes (new features, bug fixes visible to users, UI changes, behavior changes). For CI, maintenance, internal refactoring, build tooling, or other non-user-facing work, write "NONE". --> ```release-note Cherry Studio 1.8.2 - Bug Fixes and Model Updates 🐛 Bug Fixes - [Knowledge Base] Fix knowledge base content not being delivered to the model when selected in conversation - [Search] Fix crash when clicking topic title in message search results - [Backup] Fix backup restoration path issue ✨ New Features - [Models] Add support for Xiaomi MiMo-V2-Pro and MiMo-V2-Omni models with reasoning control and tool use capabilities 💄 Improvements - [Models] Upgrade MiniMax default model to M2.7 with enhanced reasoning and coding capabilities ``` Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
What this PR does
Before this PR:
The API server used raw provider
apiHostvalues from Redux without formatting. This meant provider-specific URL transformations (e.g., appending/v1, Ollama/apisuffix, Azure/openaisuffix) were skipped, causing upstream API calls to hit incorrect URLs (often resulting in 404 errors). Additionally, shared utility functions (API host formatting, provider type checks, Claude provider constants) were tightly coupled to the renderer process and unavailable to the main process.After this PR:
formatApiHost,withoutTrailingSlash,hasAPIVersion, etc.) are moved topackages/shared/utils/api/isAnthropicProvider,isOllamaProvider, etc.) are moved topackages/shared/aiCore/provider/utils/formatOllamaApiHost,formatAzureOpenAIApiHost) are moved topackages/shared/aiCore/provider/utils/api.tsCLAUDE_SUPPORTED_PROVIDERS/CLAUDE_OFFICIAL_SUPPORTED_PROVIDERSconstants are moved topackages/shared/config/providers.tsformatProviderApiHostis created atsrc/main/aiCore/provider/providerConfig.tsFixes #13192
Why we need it and why it was done in this way
The following tradeoffs were made:
formatProviderApiHostimplementation exists in the main process (src/main/aiCore/provider/providerConfig.ts) alongside the renderer version. This is because the Vertex API host formatter in the renderer depends on the Redux store directly, while the main process version usesreduxService. A// WARNINGcomment is added to remind developers to keep them in sync.The following alternatives were considered:
formatProviderApiHostwas considered but deferred due to the Vertex provider's dependency on different store access patterns ([Bug]: VertexAI Provider Data Storage Architecture Flaw - Type Mismatch with Actual Storage #13194).Breaking changes
None. All existing renderer imports continue to work via re-exports.
Special notes for your reviewer
src/main/apiServer/utils/index.ts— providers are now formatted viaformatProviderApiHost()before being cached.Revert "refactor: Add const assertions to provider arrays"commit reverts an earlier attempt that caused issues.Checklist
/gh-pr-review,gh pr diff, or GitHub UI) before requesting review from othersRelease note