diff --git a/changelog.d/fixes/12358-custom-node-api-type-precedence.md b/changelog.d/fixes/12358-custom-node-api-type-precedence.md new file mode 100644 index 0000000000..739017ad26 --- /dev/null +++ b/changelog.d/fixes/12358-custom-node-api-type-precedence.md @@ -0,0 +1 @@ +- **fix(routing):** custom OpenAI-compatible nodes now honor the saved Chat/Responses API type after edits instead of letting the node's original ID prefix override the live connection setting ([#11884](https://github.com/diegosouzapw/OmniRoute/issues/11884)). diff --git a/config/quality/file-size-baseline.json b/config/quality/file-size-baseline.json index f855229516..0b46fbf4a6 100644 --- a/config/quality/file-size-baseline.json +++ b/config/quality/file-size-baseline.json @@ -1,4 +1,5 @@ { + "_rebaseline_2026_09_11_12358_chat_pipeline_custom_node": "PR #12358 own test growth: tests/integration/chat-pipeline.test.ts 1648->1736 (+88). One new integration case, \"#11884 chat pipeline sends a custom node's edited Chat API type upstream\": it seeds a custom OpenAI-compatible node with an edited Chat/Responses API type, stubs fetch, drives handleChatCore and asserts the upstream request carries the live connection setting rather than the format baked into the node id at creation. Irreducible at this layer — the point of the test is the full route-to-upstream path, which is what #11884 regressed. Nothing else in the file changed. Covered by the case itself plus tests/unit/chat-helpers.test.ts (28/28).", "_rebaseline_2026_09_10_12975_rotation_correlation_id": "PR #12975 own growth: open-sse/executors/base.ts 1751->1753 (+2) and open-sse/handlers/chatCore.ts 6021->6024 (+3). The opencode rotation lines carry the request correlationId: one optional ExecuteInput field and one correlationId argument at each of the three executor.execute call sites in handleChatCore. Irreducible plumbing at existing call sites; the rotation logic itself lives in open-sse/executors/opencode.ts and the new leaf predicates (under cap). Covered by tests/unit/opencode-transient-rotation.test.ts and tests/unit/chat-correlation-id-exhaustion.test.ts.", "_rebaseline_2026_09_11_mergebatch_v3851_maxmad_opencode": "/merge-batch 2026-09-11 (v3.8.51), PRs #13141, #13146 and #12975 by maxmad64bis. src/sse/services/auth.ts 3450->3488 (+38): #13146 adds the narrow ruleScope===model branch to markAccountUnavailable (gated on status 400; every other status keeps its path) plus the HONORS_RULE_LOCK_SCOPE_PROVIDERS opencode entry, taking it to 3464; #12975 then adds buildExhaustionOptions so the exhaustion log lines carry the request correlationId (+24). open-sse/services/accountFallback.ts 2467->2468 (+1): #13141 routes hasFutureRateLimitUntil through the tolerant epoch normalizer; #13146 is net zero there (+16/-16). open-sse/executors/base.ts 1751->1753 (+2): #12975 adds the optional ExecuteInput.correlationId field with its doc comment. src/sse/handlers/chat.ts is NOT rebaselined: #12975 threads correlationId through the three executor call sites (+2) but the file lands at 2452, still under its existing 2458 freeze. open-sse/utils/stream.ts is deliberately NOT rebaselined either: it is already 3115 > 3098 on the pure tip with zero contribution from this batch (base-red #12732, owned by /sweep-reds). No new branching beyond the two guarded branches named above. Covered by tests/unit/combo-predicates-epoch-cooldown.test.ts, opencode-400-model-unavailable.test.ts, agentrouter-error-rules.test.ts, opencode-transient-rotation.test.ts and chat-correlation-id-exhaustion.test.ts.", "_rebaseline_2026_09_10_mergebatch_v3851_greenpt_eurouter": "/merge-batch 2026-09-10 (v3.8.51), PRs #13024 (GreenPT, closes #12986) and #13025 (EURouter, closes #12985) by ntdatt812: src/shared/constants/providers/apikey/gateways.ts 1462->1502 (+40 = two APIKEY_PROVIDERS_GATEWAYS catalog entries, declarative data only: id/alias/name/icon/color/website plus the hasFree=false rationale comments and the apiHint copy each PR verified). No logic and no new branching. Same god-file no-split rationale as every prior gateways.ts rebaseline (#11786 seekai, #10987 logfare, #10668 tabitoken, #10531 freebuff, #11631 1min.ai): the file header says it is pure data merged by apikey/index.ts via spread, and it is already split into 6 family files under apikey/, so splitting a catalog for two entries would violate the semantic-families rule rather than help. Both entries are deliberately conservative (models: [] with passthroughModels, no tool/vision capability declared, hasFree false), so the growth is the entry itself, not claims. EURouter is in AGGREGATOR_PROVIDER_IDS because it routes to third-party upstreams; GreenPT is not because it serves its own inference. Covered by tests/unit/greenpt-provider.test.ts and tests/unit/eurouter-provider.test.ts.", @@ -220,7 +221,7 @@ "_rebaseline_2026_08_24_video_bridge_fu01_fu03_fu04_result_cache_tests": "PRs #11362 (FU-01 cache hardening) + #11382 (FU-03 visual dedup policy identity) + #11383 (FU-04 focused analysis mode) own test growth: videoBridgeResultCache.test.ts <1000->1040, +40 (sum of three stacked PRs boarded together in the same merge-batch, each adding its own cache-identity assertions on the shared result-cache seam). Owner pre-authorized rebaseline for legitimate PR growth (2026-08-19 directive).", "_rebaseline_basered_codebuddy_cn": "Base-red fix (#4664 CodeBuddy CN): oauth-providers-config.test.ts 867->870 (+3) to align the EXPECTED provider list/config with the codebuddy-cn provider that #4664 added to the registry without updating this test (it asserts 'exactly once').", "_rebaseline_pr4613_compatible_provider_groups": "Reconcile #4613 already-merged growth: providers-page-utils.test.ts 1004->1052 (+48, buildCompatibleProviderGroups partition unit test). Fast-gate PR->release does not run check:file-size, so this surfaced post-merge.", - "tests/integration/chat-pipeline.test.ts": 1648, + "tests/integration/chat-pipeline.test.ts": 1736, "tests/unit/account-fallback-service.test.ts": 2056, "tests/unit/batch_api.test.ts": 1345, "tests/unit/cc-compatible-provider.test.ts": 1225, diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 0a5b3fafbe..fcd565133a 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -1489,6 +1489,7 @@ async function handleSingleModelChat( model, sourceFormat, targetFormat, + customModelTargetFormat, extendedContext, apiFormat, } = resolved; @@ -1940,7 +1941,11 @@ async function handleSingleModelChat( runtimeOptions.comboExecutionKey ?? runtimeOptions.comboStepId ?? null, extendedContext, modelApiFormat: apiFormat, - modelTargetFormat: targetFormat, + // Only a model's explicit DB override may cross this boundary as + // modelInfo.targetFormat. The effective targetFormat above was + // resolved without credentials; forwarding it would let a stale + // provider-id fallback override the credential-aware resolution. + modelTargetFormat: customModelTargetFormat, providerProfile, cachedSettings: runtimeOptions.cachedSettings, skipUpstreamRetry: runtimeOptions.skipUpstreamRetry ?? false, diff --git a/src/sse/handlers/chatHelpers.ts b/src/sse/handlers/chatHelpers.ts index 17e3a0f08a..c29300b30d 100644 --- a/src/sse/handlers/chatHelpers.ts +++ b/src/sse/handlers/chatHelpers.ts @@ -338,7 +338,15 @@ export async function resolveModelOrError( log.info("ROUTING", `Provider: ${provider}, Model: ${model}${ctxTag}`); } - return { provider, model, sourceFormat, targetFormat, extendedContext, apiFormat }; + return { + provider, + model, + sourceFormat, + targetFormat, + customModelTargetFormat, + extendedContext, + apiFormat, + }; } export async function checkPipelineGates( diff --git a/tests/integration/chat-pipeline.test.ts b/tests/integration/chat-pipeline.test.ts index 191462d93c..e2303c86b9 100644 --- a/tests/integration/chat-pipeline.test.ts +++ b/tests/integration/chat-pipeline.test.ts @@ -21,6 +21,7 @@ const { skillRegistry } = await import("../../src/lib/skills/registry.ts"); const { skillExecutor } = await import("../../src/lib/skills/executor.ts"); const { encodeSkillToolName } = await import("../../src/lib/skills/injection.ts"); const { handleChat } = await import("../../src/sse/handlers/chat.ts"); +const providerNodeRoute = await import("../../src/app/api/provider-nodes/[id]/route.ts"); const { initTranslators } = await import("../../open-sse/translator/index.ts"); const { clearInflight } = await import("../../open-sse/services/requestDedup.ts"); const { setCliCompatProviders } = await import("../../open-sse/config/cliFingerprints.ts"); @@ -550,6 +551,93 @@ test("chat pipeline handles OpenAI passthrough with valid API key auth", async ( assert.equal(json.choices[0].message.content, "OpenAI passthrough"); }); +test("#11884 chat pipeline sends a custom node's edited Chat API type upstream", async () => { + // Mirror POST /api/provider-nodes: the generated node id embeds the API type chosen at + // creation time, so a node created as Responses keeps "responses" in its id forever. + const providerId = "openai-compatible-responses-11884"; + const prefix = "edited-node-11884"; + const baseUrl = "https://edited-node-11884.example.invalid/v1"; + const nodeName = "Edited node 11884"; + await providersDb.createProviderNode({ + id: providerId, + type: "openai-compatible", + name: nodeName, + prefix, + apiType: "responses", + baseUrl, + }); + await seedConnection(providerId, { + apiKey: "sk-edited-node-11884", + providerSpecificData: { baseUrl, apiType: "responses" }, + }); + + // The operator edits the node from Responses to Chat through the real route, which also + // rewrites the connection's saved apiType. + const editResponse = await providerNodeRoute.PUT( + new Request(`http://localhost/api/provider-nodes/${providerId}`, { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ name: nodeName, prefix, apiType: "chat", baseUrl }), + }), + { params: Promise.resolve({ id: providerId }) } + ); + assert.equal(editResponse.status, 200); + const [connection] = (await providersDb.getProviderConnections({ + provider: providerId, + })) as Array<{ + providerSpecificData?: { apiType?: unknown }; + }>; + assert.equal(connection?.providerSpecificData?.apiType, "chat"); + + const apiKey = await seedApiKey(); + const fetchCalls: FetchCall[] = []; + globalThis.fetch = async (url, init: RequestInit = {}) => { + const call: FetchCall = { + url: String(url), + method: init.method || "GET", + headers: toPlainHeaders(init.headers), + body: init.body ? JSON.parse(String(init.body)) : null, + }; + fetchCalls.push(call); + if (!call.url.startsWith(baseUrl)) { + throw new Error(`unexpected upstream call: ${call.method} ${call.url}`); + } + return buildOpenAIResponse("Edited node reply", "edited-model"); + }; + + const response = await handleChat( + buildRequest({ + authKey: apiKey.key, + body: { + model: `${prefix}/edited-model`, + stream: false, + messages: [{ role: "user", content: "Hello edited node" }], + }, + }) + ); + + const json = (await response.json()) as { choices: Array<{ message: { content: string } }> }; + assert.ok(fetchCalls.length >= 1, "expected an upstream request"); + const upstream = fetchCalls[0]; + assert.equal(upstream.method, "POST"); + assert.equal(upstream.url, `${baseUrl}/chat/completions`); + assert.equal(upstream.headers.Authorization, "Bearer sk-edited-node-11884"); + assert.deepEqual( + upstream.body.messages, + [{ role: "user", content: "Hello edited node" }], + "the saved Chat API type must produce a Chat Completions body" + ); + assert.equal( + upstream.body.input, + undefined, + "the stale Responses API type from the node id must not shape the upstream body" + ); + assert.equal(upstream.body.model, "edited-model"); + assert.equal(fetchCalls.length, 1, "exactly one upstream request"); + assert.equal(response.status, 200); + assert.equal(json.choices[0].message.content, "Edited node reply"); +}); + test("chat pipeline persists Codex responses cache and reasoning tokens to call logs", async () => { await seedConnection("codex", { apiKey: "sk-codex-primary" }); const fetchCalls = []; diff --git a/tests/unit/chat-helpers.test.ts b/tests/unit/chat-helpers.test.ts index 710bb6f21c..21ac50fa27 100644 --- a/tests/unit/chat-helpers.test.ts +++ b/tests/unit/chat-helpers.test.ts @@ -24,6 +24,9 @@ const { getCircuitBreaker, resetAllCircuitBreakers, STATE } = await import("../../src/shared/utils/circuitBreaker.ts"); // DATA_DIR must be fixed before these modules load; keep this test seam dynamic. const { setTlsClientForTest } = await import("../../open-sse/utils/proxyFetch.ts"); +const { resolveChatCoreTargetFormat } = + await import("../../open-sse/handlers/chatCore/targetFormat.ts"); +const { FORMATS } = await import("../../open-sse/translator/formats.ts"); type ApiErrorJson = { error?: { @@ -259,6 +262,59 @@ test("resolveModelOrError honors a custom-model targetFormat override even when assert.equal(result.targetFormat, "claude"); }); +test("#11884 configured Chat API type wins after custom-node model resolution", async () => { + const provider = "openai-compatible-responses-11884"; + const prefix = "custom-chat-11884"; + const model = "chat-only-model"; + + await providersDb.createProviderNode({ + id: provider, + type: "openai-compatible", + name: "Custom Chat 11884", + prefix, + apiType: "chat", + baseUrl: "https://chat-only.example.invalid/v1", + }); + const connection = await seedConnection(provider, { + providerSpecificData: { apiType: "chat" }, + }); + const modelsDb = await import("../../src/lib/db/models.ts"); + await modelsDb.addCustomModel(provider, model, "Chat-only model", "manual", "chat-completions", [ + "chat", + ]); + + const firstResolution = await resolveModelOrError( + `${prefix}/${model}`, + { model: `${prefix}/${model}`, messages: [{ role: "user", content: "hello" }] }, + "/v1/chat/completions" + ); + assert.equal(firstResolution.error, undefined); + + // Before #11884's fix the resolver exposed only its credential-blind effective + // targetFormat, so the dispatcher necessarily forwarded that value as though it + // were a model override. The fixed contract exposes the explicit model override + // separately; keep the fallback here so this regression test still exercises the + // broken production path when run against the parent revision. + const forwardedModelOverride = + "customModelTargetFormat" in firstResolution + ? firstResolution.customModelTargetFormat + : firstResolution.targetFormat; + const finalResolution = resolveChatCoreTargetFormat({ + provider: firstResolution.provider, + resolvedModel: firstResolution.model, + apiFormat: firstResolution.apiFormat, + sourceFormat: firstResolution.sourceFormat, + customModelTargetFormat: forwardedModelOverride, + providerSpecificData: connection.providerSpecificData, + }); + + assert.equal( + finalResolution.targetFormat, + FORMATS.OPENAI, + "the stored Chat API type must not be shadowed by a stale Responses fallback" + ); +}); + test("checkPipelineGates blocks providers with an open circuit breaker", async () => { const breaker = getCircuitBreaker("openai"); breaker.state = STATE.OPEN;