mirror of
https://github.com/nexu-io/open-design.git
synced 2026-10-27 09:51:12 +01:00
fix(memory): forward BYOK chat model through /api/memory/extract (#5177)
* fix: forward BYOK chat model to memory extraction (#5162) Root cause: Two issues preventing non-OpenAI BYOK providers (e.g. MiniMax) from working with memory-llm extraction: 1. Web side (ProjectView.tsx): The BYOK chatProvider snapshot sent to /api/memory/extract was missing the `model` field. Without it, pickProvider() fell through to PROVIDER_DEFAULTS.openai.model ('gpt-4o-mini'), which doesn't exist on non-OpenAI endpoints. 2. Daemon route (memory.ts): The /api/memory/extract POST handler did not extract body.chatModel, unlike every other memory route. Even when the web side passes a model, it was silently dropped. Changes: - ProjectView.tsx: Add `model: byokOpenCodeProvider.model` to the BYOK chatProvider snapshot so pickProvider() can use the user's actual chat model (e.g. 'MiniMax-M3'). - routes/memory.ts: Read `body.chatModel` and forward it to extractWithLLM options, matching the pattern used by all other memory routes. Testing: typecheck passes for apps/web (affected packages). The daemon typecheck requires pre-built @open-design/release which is a separate toolchain concern. * fix(contracts): add model field to ByokChatProviderConfig for BYOK memory extraction (#5177) The web-side memory snapshot was reading byokOpenCodeProvider.model but the BYOK config contract had no model property, causing a compile-time type error. Add an optional model field so the daemon can forward the user's chat-model picker choice to memory extraction. * fix(byok): copy config.model into byokOpenCodeProvider snapshot Address review comment on PR #5177: byokOpenCodeProviderFromConfig() did not copy config.model into the returned ByokChatProviderConfig, so byokChatProvider.model serialized as undefined and /api/memory/extract fell back to the hardcoded default (gpt-4o-mini) for OpenAI-compatible BYOK endpoints like MiniMax. Threading config.model through ensures the chat-selected model reaches memory extraction. Reviewed-by: mrcfps (review comments on ProjectView.tsx:5049) * test: add model field to BYOK payload expectations in web workspace tests * fix(daemon): forward BYOK chatProvider to post-turn memory extraction The daemon close handler creates memoryOptions for pickProvider() but never forwarded the per-call BYOK provider snapshot ('byokProvider' from the chat request body). This meant post-turn LLM memory extraction always fell through to environment/default provider detection instead of using the user's selected BYOK provider and endpoint. Added 'chatProvider: byokProvider ?? null' to the memoryOptions object so pickProvider() can run 'Same as chat' extraction against the user's actual provider/config, matching the behavior of the pre-turn /api/memory/extract path. Addresses reviewer feedback on PR #5177 (fix #5162). * fix(daemon): normalize byokProvider shape for post-turn memory extraction (#5177) The close handler forwarded byokProvider ({ protocol, ... }) directly as chatProvider, but memory-llm.pickProvider expects { provider, ... } with provider being a PROVIDER_DEFAULTS key. Now we map protocol → provider the same way the web pre-turn path does (ProjectView.tsx L5894). * fix(daemon): support keyless BYOK endpoints in post-turn memory extraction (#5177) Three-part fix for keyless BYOK (local vLLM / Ollama / openai-compatible servers with requiresApiKey=false): 1. server.ts normalization: thread requiresApiKey through to the memoryChatProvider snapshot so pickProvider() can distinguish keyless endpoints from missing-provider cases. 2. memory-llm pickProvider() BYOK gate: relax the apiKey guard so endpoints explicitly marked with requiresApiKey=false enter the BYOK branch instead of falling through to env/media-config fallbacks (which would route to unrelated OpenAI credentials / gpt-4o-mini). 3. memory-llm callOpenAI(): conditionally emit Authorization header only when apiKey is non-empty, preventing empty Bearer from being sent to keyless local servers that would reject it.
This commit is contained in:
@@ -489,6 +489,14 @@ async function pickProvider(projectRoot, dataDir, chatAgentId, chatProvider, cha
|
||||
// big chat model (gpt-4o, claude-sonnet-4-5) silently turns into a
|
||||
// cheap haiku/mini call. The caller can opt into using the chat
|
||||
// model verbatim by setting `chatProvider.model`.
|
||||
//
|
||||
// Keyless BYOK (local vLLM / Ollama / openai-compatible servers with
|
||||
// `requiresApiKey: false`) is also valid — the web app explicitly
|
||||
// marks it via `requiresApiKey: false` on the snapshot. We must
|
||||
// enter the BYOK branch in that case too because the alternative
|
||||
// paths below all require an env / media-config key and would fall
|
||||
// back to unrelated OpenAI credentials (the gpt-4o-mini default this
|
||||
// hook exists to avoid), defeating the BYOK guarantee.
|
||||
if (
|
||||
chatProvider
|
||||
&& chatProvider.provider
|
||||
@@ -496,13 +504,14 @@ async function pickProvider(projectRoot, dataDir, chatAgentId, chatProvider, cha
|
||||
) {
|
||||
const apiKey =
|
||||
typeof chatProvider.apiKey === 'string' ? chatProvider.apiKey.trim() : '';
|
||||
if (apiKey) {
|
||||
const allowKeyless = chatProvider.requiresApiKey === false;
|
||||
if (apiKey || allowKeyless) {
|
||||
const defaults = PROVIDER_DEFAULTS[chatProvider.provider];
|
||||
const baseUrl =
|
||||
(typeof chatProvider.baseUrl === 'string' && chatProvider.baseUrl.trim())
|
||||
|| defaults.baseUrl;
|
||||
// Azure with no resource URL is unrecoverable — same guard as
|
||||
// the override path above.
|
||||
// the override path above. (Azure is never keyless.)
|
||||
if (chatProvider.provider !== 'azure' || baseUrl) {
|
||||
const explicitModel =
|
||||
typeof chatProvider.model === 'string' && chatProvider.model.trim()
|
||||
@@ -520,6 +529,10 @@ async function pickProvider(projectRoot, dataDir, chatAgentId, chatProvider, cha
|
||||
|| PROVIDER_DEFAULTS.azure.apiVersion
|
||||
: '',
|
||||
credentialSource: 'chat-byok',
|
||||
// Preserve the keyless signal for the HTTP call layer so it
|
||||
// omits the Authorization header instead of sending `Bearer `
|
||||
// (empty) which the local server would reject.
|
||||
...(allowKeyless ? { requiresApiKey: false } : {}),
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -716,7 +729,14 @@ async function callOpenAI(provider, system, user) {
|
||||
method: 'POST',
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
authorization: `Bearer ${provider.apiKey}`,
|
||||
// Keyless BYOK endpoints (local vLLM / Ollama / openai-compatible
|
||||
// servers marked with `requiresApiKey: false`) accept requests
|
||||
// without an Authorization header — sending `Bearer ` (empty)
|
||||
// would cause some servers to reject the call. Only attach the
|
||||
// header when we actually have a key.
|
||||
...(provider.apiKey
|
||||
? { authorization: `Bearer ${provider.apiKey}` }
|
||||
: {}),
|
||||
// AIHubMix routes through this same OpenAI-compatible path but wants
|
||||
// the fixed APP-Code attribution header on every request.
|
||||
...(provider.kind === 'aihubmix' && AIHUBMIX_APP_CODE
|
||||
|
||||
@@ -588,6 +588,10 @@ export function registerMemoryRoutes(app: Express, ctx: RegisterMemoryRoutesDeps
|
||||
};
|
||||
}
|
||||
}
|
||||
const chatModel =
|
||||
typeof body.chatModel === 'string' && body.chatModel.trim()
|
||||
? body.chatModel.trim()
|
||||
: '';
|
||||
let attemptedLLM = false;
|
||||
if (userMessage.trim().length > 0 && hasAssistant) {
|
||||
attemptedLLM = true;
|
||||
@@ -600,6 +604,7 @@ export function registerMemoryRoutes(app: Express, ctx: RegisterMemoryRoutesDeps
|
||||
projectRoot: PROJECT_ROOT,
|
||||
chatAgentId: null,
|
||||
chatProvider,
|
||||
...(chatModel ? { chatModel } : {}),
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
@@ -6444,10 +6444,38 @@ export async function startServer({
|
||||
// a Claude Code (anthropic) chat from triggering OpenAI/gpt-4o-
|
||||
// mini extraction in the background just because the user has
|
||||
// an OpenAI key parked in media-config.
|
||||
//
|
||||
// Also normalize the BYOK provider shape: web side sends
|
||||
// `{ protocol, ... }` via the chat body as `byokProvider`,
|
||||
// but memory-llm.pickProvider expects `{ provider, ... }`
|
||||
// with `provider` being a PROVIDER_DEFAULTS key. We apply the
|
||||
// same mapping the web pre-turn path does (ProjectView.tsx
|
||||
// constructs `{ provider: byokOpenCodeProvider.protocol, ... }`).
|
||||
const memoryChatProvider: {
|
||||
provider?: string;
|
||||
apiKey?: string;
|
||||
baseUrl?: string;
|
||||
apiVersion?: string;
|
||||
model?: string;
|
||||
requiresApiKey?: boolean;
|
||||
} | null = byokProvider
|
||||
? {
|
||||
provider: (byokProvider as { protocol?: string }).protocol ?? undefined,
|
||||
apiKey: (byokProvider as { apiKey?: string }).apiKey,
|
||||
baseUrl: (byokProvider as { baseUrl?: string }).baseUrl,
|
||||
apiVersion: (byokProvider as { apiVersion?: string }).apiVersion,
|
||||
model: (byokProvider as { model?: string }).model,
|
||||
requiresApiKey: (byokProvider as { requiresApiKey?: boolean }).requiresApiKey,
|
||||
}
|
||||
: null;
|
||||
const memoryOptions = {
|
||||
projectRoot: PROJECT_ROOT,
|
||||
chatAgentId: typeof agentId === 'string' ? agentId : null,
|
||||
chatModel: typeof safeModel === 'string' ? safeModel : null,
|
||||
// Forward the per-call BYOK provider snapshot so pickProvider()
|
||||
// can run "Same as chat" extraction against the user's actual
|
||||
// provider/endpoint/model instead of falling back to defaults.
|
||||
chatProvider: memoryChatProvider,
|
||||
// Scope the extractor's duplicate-turn de-dup to this conversation, so a
|
||||
// re-fired turn collapses but an identical (message, reply) in another
|
||||
// conversation is still examined.
|
||||
|
||||
@@ -1254,6 +1254,7 @@ function byokOpenCodeProviderFromConfig(
|
||||
protocol: config.apiProtocol,
|
||||
apiKey: config.apiKey.trim(),
|
||||
baseUrl: config.baseUrl,
|
||||
model: config.model,
|
||||
...(selectedProvider?.requiresApiKey === false ? { requiresApiKey: false } : {}),
|
||||
apiVersion:
|
||||
config.apiProtocol === 'azure'
|
||||
@@ -6128,6 +6129,7 @@ export function ProjectView({
|
||||
apiKey: byokOpenCodeProvider.apiKey,
|
||||
baseUrl: byokOpenCodeProvider.baseUrl,
|
||||
apiVersion: byokOpenCodeProvider.apiVersion,
|
||||
model: byokOpenCodeProvider.model,
|
||||
}
|
||||
: undefined;
|
||||
if (userText.length > 0) {
|
||||
|
||||
@@ -1811,6 +1811,7 @@ describe('ProjectView conversation run isolation', () => {
|
||||
baseUrl: 'http://localhost:11434',
|
||||
requiresApiKey: false,
|
||||
apiVersion: '',
|
||||
model: 'llama3.2',
|
||||
},
|
||||
model: 'llama3.2',
|
||||
}));
|
||||
@@ -1844,6 +1845,7 @@ describe('ProjectView conversation run isolation', () => {
|
||||
baseUrl: 'http://127.0.0.1:8000/v1',
|
||||
requiresApiKey: false,
|
||||
apiVersion: '',
|
||||
model: 'model',
|
||||
},
|
||||
model: 'model',
|
||||
}));
|
||||
|
||||
@@ -46,6 +46,13 @@ export interface ByokChatProviderConfig {
|
||||
apiVersion?: string;
|
||||
/** Explicit run-scoped provider policy for presets that do not require bearer credentials. */
|
||||
requiresApiKey?: boolean;
|
||||
/**
|
||||
* Run-scoped chat model id selected in the chat UI. Forwarded to the daemon
|
||||
* so BYOK-backed utilities (e.g. memory extraction) can honor the user's
|
||||
* model picker instead of falling back to a hardcoded default. Optional
|
||||
* because some presets (e.g. Ollama) infer the model from baseUrl/protocol.
|
||||
*/
|
||||
model?: string;
|
||||
}
|
||||
|
||||
export interface ByokMediaDefaults {
|
||||
|
||||
Reference in New Issue
Block a user