Skip to content

fix: inject custom provider params at fetch layer#16102

Open
Raymond8196 wants to merge 1 commit into
CherryHQ:v1from
Raymond8196:hotfix/custom-params-dropped-clean
Open

fix: inject custom provider params at fetch layer#16102
Raymond8196 wants to merge 1 commit into
CherryHQ:v1from
Raymond8196:hotfix/custom-params-dropped-clean

Conversation

@Raymond8196

Copy link
Copy Markdown
Contributor

What this PR does

Before this PR:
Assistant custom parameters were merged into AI SDK providerOptions, but many AI SDK provider adapters stripped unrecognized provider options before serializing the HTTP request body. As a result, user-defined params such as store: false, n, or JSON params could silently disappear from chat requests.

After this PR:
Cherry Studio carries filtered custom provider params through the chat parameter pipeline and wraps the provider fetch function to inject them into the serialized JSON request body at low precedence. SDK-generated body fields still win, and provider-scoped option objects are not injected as top-level HTTP body fields.

Fixes #16041

Why we need it and why it was done in this way

The following tradeoffs were made:
The fix operates at the fetch layer so it works across providers whose AI SDK adapters strip unknown provider options. It only injects flat body passthrough params and leaves provider-scoped option objects in providerOptions.

The following alternatives were considered:
Passing unknown fields through providerOptions was not sufficient because adapters strip them. Provider-specific adapter patches were avoided because they would repeat the same bug for every provider and parameter.

Links to places where the discussion took place: #16041

Breaking changes

None.

Special notes for your reviewer

The wrapper merges custom params with low precedence ({ ...customParams, ...body }) so SDK-computed body fields are preserved. Tests cover raw snake_case passthrough, provider-scoped object exclusion, invalid JSON/non-string body passthrough, and existing fetch wrapper chaining.

Verification run locally:

  • pnpm format
  • pnpm lint
  • pnpm test

Checklist

  • PR: The PR description is detailed enough to allow reviewers to understand the change. It includes a summary, context, and any relevant links.
  • Code: The code is well-structured, readable, and maintainable. It includes appropriate tests, documentation, and error handling where necessary.
  • Refactor: This PR includes code cleanup or restructuring without changing behavior.
  • Upgrade: If the PR changes the data model, storage schema, or persistent state, the migration logic is included and tested. If not applicable, this has been considered.
  • Documentation: A user-guide update 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 performed a self-review of my code and tested the change locally.

Release note

Fix assistant custom parameters being silently dropped by AI SDK provider adapters.

Signed-off-by: raymond <13162938362@163.com>
@DeJeune DeJeune self-assigned this Jun 18, 2026

@DeJeune DeJeune left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fix: inject custom provider params at fetch layer

Thanks for the fix — the fetch-layer re-injection approach is sound and the core mechanics (low-precedence merge, non-JSON body passthrough, signature-fetch ordering) are correct and well-tested. I found one blocking reliability issue plus a couple of lower-severity notes.

Decision: 🔴 Request changes (the high-severity issue below should be resolved before merge).

🔴 Blocking — provider-instance cache can serve a stale / incorrect wrapped fetch

(See the inline comment on AiProvider.ts for the anchored code.)

Built provider instances are cached in a module-level LRU keyed by computeHash(mergedSettings) in packages/aiCore/src/core/providers/core/ProviderExtension.ts, and stableStringify collapses every function to the constant string "[function]" (~line 166). The fetch wrapped here now behaves differently depending on customProviderParams, but customProviderParams is not part of the hashed settings.

Consequence: two requests with the same provider + model + apiKey but different custom params hash to the same cache key and reuse the earlier cached instance, which closed over the earlier request's fetch. So custom params can be silently dropped or cross-contaminated between assistants/requests.

  • Standard providers: collides between two requests that both carry (different) custom params.
  • CherryIN / openai-compatible / other proxy providers that always carry a signature fetch: collide even between an empty-params and a non-empty-params request — i.e. exactly the proxy providers this PR is meant to help.

Verified end-to-end: AiProvider.modernCompletionscreateExecutorExtensionRegistry.createProviderProviderExtension.createProvider (LRU get(hash)); the provider closes over settings.fetch at construction and RuntimeExecutor does not re-resolve it per request; the registry is a module-level singleton, so the cache persists for the session.

Suggested directions (any one):

  1. Fold a stable hash of customProviderParams into the hashed settings so the cache key differs.
  2. Bypass the instance cache when customProviderParams is non-empty.
  3. Inject via a per-request middleware/plugin instead of the cached provider's fetch.

🟡 Test coverage

  • No integration test exercises AiProvider reading middlewareConfig.customProviderParams, the empty-params no-op branch, or chaining onto an existing providerSettings.fetch. A test here would also have surfaced the cache issue above. (The producer/forwarder/wrapper — buildProviderOptions, buildStreamTextParams, createCustomParamsFetch — are each covered in isolation.)
  • parameterBuilder.test.ts mocks ../../utils/options and then asserts the passthrough, so it mainly verifies plumbing. The real extraction is covered by the new options.test.ts cases, so this one is minor.

ℹ️ Minor

isProviderScopedParam only excludes a provider-namespace key when its value is an object, so malformed input like openai: "foo" would leak as a stray top-level body field. Low impact (requires nonsensical input); optional hardening.

✅ Verified correct (no change needed)

  • Precedence: { ...customParams, ...body } correctly lets SDK-computed body values win on key conflict.
  • Security: model / messages / stream etc. cannot be overridden; non-JSON bodies (FormData / Blob / stream) and invalid JSON are passed through untouched; the wrapper is scoped per-completion (no global fetch pollution).
  • Wrapper ordering: custom-params fetch is the outer layer, CherryAI signature fetch the inner — so the signature is computed over the final injected body. ✔
  • reasoning_effort snapshot (taken before the snake→camel conversion) is not a double-send: the openai-compatible adapter emits reasoning_effort itself and wins the low-precedence merge.

Note for other reviewers

An earlier review pass flagged the summary/topic-naming path (ApiService.ts fetchMessagesSummary) for dropping customProviderParams. That is a non-issue: the summary assistant is built from getDefaultAssistant(), whose DEFAULT_ASSISTANT_SETTINGS.customParameters is always [], so customProviderParams is empty on that path and omitting it has no behavioral effect.

if (middlewareConfig.customProviderParams && Object.keys(middlewareConfig.customProviderParams).length > 0) {
const existingFetch = (providerSettings as Record<string, any>).fetch ?? globalThis.fetch
const wrappedFetch = createCustomParamsFetch(existingFetch, middlewareConfig.customProviderParams)
providerSettings = { ...providerSettings, fetch: wrappedFetch }

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking — this cached provider can be served with a stale wrapped fetch.

Built providers are cached in a module-level LRU keyed by computeHash(mergedSettings) (packages/aiCore/src/core/providers/core/ProviderExtension.ts), and stableStringify serializes every function as the constant "[function]" (~line 166). The fetch wrapped here now behaves differently depending on customProviderParams, but that value is not part of the hashed settings.

So two requests with the same provider + model + apiKey but different custom params (and, for providers that always carry a signature fetch like CherryIN / openai-compatible, even empty-vs-non-empty) hash to the same key and reuse the earlier instance — which closed over the earlier request's fetch. Custom params can be silently dropped or cross-contaminated between assistants.

Options: fold a stable hash of customProviderParams into the hashed settings, bypass the instance cache when it is non-empty, or inject via a per-request middleware/plugin rather than the cached provider's fetch.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants