From 0a8b296d5380699db692e47ff14f2232917d8648 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:24:39 -0300 Subject: [PATCH 1/2] fix(db): keep proxyLogger's import-time hydration off the DB module graph MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit db/proxyLogs imported normalizeProxyHostForLog from proxyLogger (#14364), and proxies/rotation started importing db/proxyLogs (#14657). Together they pulled proxyLogger — which runs loadFromDb() at import — into every module reaching settings -> proxies, so a cold getPricingForModel() paid two proxy_logs queries (pricing-for-model-cached-13891 red). Move the pure helper into a zero-import leaf (src/lib/proxyLogHost.ts); proxyLogger re-exports it for existing callers. --- src/lib/db/proxyLogs.ts | 2 +- src/lib/proxyLogHost.ts | 25 +++++++++++++++++++++++++ src/lib/proxyLogger.ts | 21 ++++----------------- 3 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 src/lib/proxyLogHost.ts diff --git a/src/lib/db/proxyLogs.ts b/src/lib/db/proxyLogs.ts index 897abc97e2ae..54f8efe2bd3a 100644 --- a/src/lib/db/proxyLogs.ts +++ b/src/lib/db/proxyLogs.ts @@ -15,7 +15,7 @@ */ import { getDbInstance } from "./core"; -import { normalizeProxyHostForLog } from "../proxyLogger"; +import { normalizeProxyHostForLog } from "../proxyLogHost"; import { sanitizeTimingMs } from "@omniroute/open-sse/utils/timingMs.ts"; // --------------------------------------------------------------------------- diff --git a/src/lib/proxyLogHost.ts b/src/lib/proxyLogHost.ts new file mode 100644 index 000000000000..ca27a20e0b1f --- /dev/null +++ b/src/lib/proxyLogHost.ts @@ -0,0 +1,25 @@ +/** + * Zero-import leaf. `src/lib/db/proxyLogs.ts` needs this helper; importing it from + * `proxyLogger.ts` loaded that module's import-time `loadFromDb()` hydration into every + * DB module that reaches proxyLogs (settings → proxies → proxies/rotation → proxyLogs), + * so a cold `getPricingForModel()` paid two extra proxy_logs queries + * (tests/unit/pricing-for-model-cached-13891.test.ts). + */ + +/** + * Canonical host normalization for proxy log writes and (host, port) lookups: + * trim, strip exactly one pair of surrounding brackets from IPv6 literals + * ("[2001:db8::1]"), re-trim, lowercase. Anything that is not a non-empty + * string normalizes to null so readers can fall back to today's behavior. + */ +export function normalizeProxyHostForLog(host: unknown): string | null { + if (typeof host !== "string") return null; + const trimmed = host.trim(); + if (!trimmed) return null; + const unbracketed = + trimmed.startsWith("[") && trimmed.endsWith("]") && trimmed.length > 2 + ? trimmed.slice(1, -1).trim() + : trimmed; + if (!unbracketed) return null; + return unbracketed.toLowerCase(); +} diff --git a/src/lib/proxyLogger.ts b/src/lib/proxyLogger.ts index 4a703691ed6c..6482c37d9879 100644 --- a/src/lib/proxyLogger.ts +++ b/src/lib/proxyLogger.ts @@ -11,24 +11,11 @@ import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/errorSanitizatio import { sanitizeTimingMs } from "@omniroute/open-sse/utils/timingMs.ts"; import { getDbInstance, isCloud, isBuildPhase } from "./db/core"; import { ensureProxyLogsColumns } from "./db/schemaColumns"; +import { normalizeProxyHostForLog } from "./proxyLogHost"; -/** - * Canonical host normalization for proxy log writes and (host, port) lookups: - * trim, strip exactly one pair of surrounding brackets from IPv6 literals - * ("[2001:db8::1]"), re-trim, lowercase. Anything that is not a non-empty - * string normalizes to null so readers can fall back to today's behavior. - */ -export function normalizeProxyHostForLog(host: unknown): string | null { - if (typeof host !== "string") return null; - const trimmed = host.trim(); - if (!trimmed) return null; - const unbracketed = - trimmed.startsWith("[") && trimmed.endsWith("]") && trimmed.length > 2 - ? trimmed.slice(1, -1).trim() - : trimmed; - if (!unbracketed) return null; - return unbracketed.toLowerCase(); -} +// Re-exported for existing callers; the helper lives in a zero-import leaf so DB modules +// can use it without loading this module (which hydrates from SQLite at import time). +export { normalizeProxyHostForLog }; const shouldPersistToDisk = !isCloud && !isBuildPhase; From f2d5ec4682a1ccb4681b1d771e7b203de01f44b5 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:25:50 -0300 Subject: [PATCH 2/2] test: align base-red tests with intentional contract changes on release/v3.8.51 - ghe-copilot: #14732 declared thinking-effort tiers on the GHE Copilot Opus 5 entry. - proxy-management-v1-route: #15044 stopped requireManagementAuth from inheriting the public-route shortcut; reset storage after the INITIAL_PASSWORD block, which persisted setupComplete=true. - audio-speech-handler: #15067 rejects dot-segment model ids at parse time; keep the handler-level HuggingFace guard covered with an id the parser accepts. - combo-provider-cooldown-sibling: #13548 moved the status >= 500 predicate into modelScopedQuotaFailure.ts; follow it in the source guard. - vision-bridge 14003: #14117 gave kimi-for-coding a vision spec, so the fixture no longer had UNKNOWN capability; switch to deepseek-coder-6.7b and pin the precondition. - build-profile-stubs: #12810 renamed the stub's installNinerouter to install and added the remaining exports; assert all of them stay disabled. --- tests/unit/audio-speech-handler.test.ts | 33 ++++++++------ .../combo-provider-cooldown-sibling.test.ts | 12 +++++- tests/unit/ghe-copilot.test.ts | 5 +++ ...ridge-bare-id-unknown-vision-14003.test.ts | 43 +++++++++++++------ tests/unit/proxy-management-v1-route.test.ts | 7 +++ .../unit/security/build-profile-stubs.test.ts | 8 +++- 6 files changed, 79 insertions(+), 29 deletions(-) diff --git a/tests/unit/audio-speech-handler.test.ts b/tests/unit/audio-speech-handler.test.ts index dfe0961d2f8d..5ca669e6d1c9 100644 --- a/tests/unit/audio-speech-handler.test.ts +++ b/tests/unit/audio-speech-handler.test.ts @@ -155,10 +155,7 @@ test("handleAudioSpeech maps OpenAI stock voice name alloy to a real ElevenLabs }); assert.equal(response.status, 200); - assert.equal( - capturedUrl, - "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM" - ); + assert.equal(capturedUrl, "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM"); } finally { globalThis.fetch = originalFetch; } @@ -187,10 +184,7 @@ test("handleAudioSpeech resolves ElevenLabs display name 'rachel' case-insensiti }); assert.equal(response.status, 200); - assert.equal( - capturedUrl, - "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM" - ); + assert.equal(capturedUrl, "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM"); } finally { globalThis.fetch = originalFetch; } @@ -218,10 +212,7 @@ test("handleAudioSpeech defaults to Rachel's voice_id when voice is omitted", as }); assert.equal(response.status, 200); - assert.equal( - capturedUrl, - "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM" - ); + assert.equal(capturedUrl, "https://api.elevenlabs.io/v1/text-to-speech/21m00Tcm4TlvDq8ikWAM"); } finally { globalThis.fetch = originalFetch; } @@ -542,13 +533,29 @@ test("handleAudioSpeech routes Nvidia TTS providers with default voice", async ( }); test("handleAudioSpeech validates HuggingFace model identifiers", async () => { - const response = await handleAudioSpeech({ + // #15067 moved dot-segment rejection into model parsing (hasUnsafeModelIdSyntax), so + // "huggingface/../escape" no longer resolves to a provider at all — still a 400. + const traversal = await handleAudioSpeech({ body: { model: "huggingface/../escape", input: "bad model", }, credentials: { apiKey: "hf-key" }, }); + const traversalPayload = (await traversal.json()) as { error: { message: string } }; + + assert.equal(traversal.status, 400); + assert.match(traversalPayload.error.message, /^No speech provider found for model/); + + // Ids the parser accepts but the HuggingFace path guard still refuses keep the + // handler-level "Invalid model ID" answer (defense in depth). + const response = await handleAudioSpeech({ + body: { + model: "huggingface/facebook//mms-tts-eng", + input: "bad model", + }, + credentials: { apiKey: "hf-key" }, + }); const payload = (await response.json()) as any; assert.equal(response.status, 400); diff --git a/tests/unit/combo-provider-cooldown-sibling.test.ts b/tests/unit/combo-provider-cooldown-sibling.test.ts index ed125bd28d69..639ff3c1149e 100644 --- a/tests/unit/combo-provider-cooldown-sibling.test.ts +++ b/tests/unit/combo-provider-cooldown-sibling.test.ts @@ -116,9 +116,17 @@ test("source guard: auth.ts skips model lockout for per-model-quota providers on path.join(process.cwd(), "src", "sse", "services", "auth.ts"), "utf-8" ); - // The fix adds an early return for status >= 500 that skips recordModelLockoutFailure + // #13548 moved the model-scoped status predicate (404 / 429 / `status >= 500`) out of + // auth.ts into modelScopedQuotaFailure.ts::isModelScopedFailure(); auth.ts must still + // route through it and keep the no-lockout early return for server errors. + const helperSrc = fs.readFileSync( + path.join(process.cwd(), "src", "sse", "services", "modelScopedQuotaFailure.ts"), + "utf-8" + ); assert.ok( - src.includes("status >= 500") && src.includes("no model lockout"), + helperSrc.includes("status >= 500") && + src.includes("isModelScopedFailure(status") && + src.includes("no model lockout"), "auth.ts must have a guard that skips model lockout for 500+ server errors on per-model-quota providers" ); // Verify the early return sends cooldownMs: 0 (no cooldown for sibling models) diff --git a/tests/unit/ghe-copilot.test.ts b/tests/unit/ghe-copilot.test.ts index 4e1a5ca0ead4..b99eaa0c5b65 100644 --- a/tests/unit/ghe-copilot.test.ts +++ b/tests/unit/ghe-copilot.test.ts @@ -13,6 +13,11 @@ test("GHE Copilot registry exposes Claude Opus 5", () => { name: "Claude Opus 5", contextLength: 1000000, maxOutputTokens: 64000, + // #14732 declared the thinking-effort tiers on the first-party Claude registries + // (GHE Copilot included) — intentional contract change. + supportsReasoning: true, + supportedThinkingEfforts: ["low", "medium", "high", "xhigh", "max"], + supportsXHighEffort: true, unsupportedParams: ["temperature", "top_p", "top_k"], }); }); diff --git a/tests/unit/guardrails/vision-bridge-bare-id-unknown-vision-14003.test.ts b/tests/unit/guardrails/vision-bridge-bare-id-unknown-vision-14003.test.ts index bbd7bd61d889..410591e05b81 100644 --- a/tests/unit/guardrails/vision-bridge-bare-id-unknown-vision-14003.test.ts +++ b/tests/unit/guardrails/vision-bridge-bare-id-unknown-vision-14003.test.ts @@ -13,12 +13,19 @@ * (`command-code/moonshotai/Kimi-K2.6`) with no error surfaced to the client: * the wrong model answered, billing followed the wrong connection, and * `call_logs` recorded the substitute as if it were intended. + * + * Fixture note: #14117 later gave `kimi-for-coding` a MODEL_SPECS entry (Kimi K2.8 + * Preview, supportsVision: true), so it no longer exercises the UNKNOWN-capability + * path. The cases below use `deepseek-coder-6.7b` (served by `llamagate`), which has + * neither a spec nor a registry vision flag; a precondition test pins that so a future + * spec change fails here loudly instead of silently turning these cases into no-ops. */ import test from "node:test"; import assert from "node:assert/strict"; const { VisionBridgeGuardrail } = await import("../../../src/lib/guardrails/visionBridge.ts"); const { resetGuardrailsForTests } = await import("../../../src/lib/guardrails/registry.ts"); +const { getResolvedModelCapabilities } = await import("../../../src/lib/modelCapabilities.ts"); import type { GuardrailContext } from "../../../src/lib/guardrails/base.ts"; import type { VisionModelConfig } from "../../../src/lib/guardrails/visionBridgeHelpers.ts"; @@ -94,24 +101,34 @@ test.beforeEach(() => { mockSettings = baseSettings(); }); +const BARE_UNKNOWN = "deepseek-coder-6.7b"; +const QUALIFIED_UNKNOWN = `llamagate/${BARE_UNKNOWN}`; + +test("VB-14003-FIXTURE: the bare and qualified fixture ids still have UNKNOWN vision", () => { + assert.strictEqual(getResolvedModelCapabilities(BARE_UNKNOWN).supportsVision, null); + const qualified = getResolvedModelCapabilities(QUALIFIED_UNKNOWN); + assert.strictEqual(qualified.provider, "llamagate"); + assert.strictEqual(qualified.supportsVision, null); +}); + test("VB-14003-BARE: keeps a bare model id with unknown vision capability instead of rerouting", async () => { - // Models production exactly: hasUsableCredentialsForModel("kimi-for-coding") + // Models production exactly (reported with `kimi-for-coding`): hasUsableCredentialsForModel(bare) // is blind to the bare id (it splits on "/" and treats the MODEL name as a // provider, finds no rows, and reports false), while the Vision Bridge's // reroute target is a perfectly healthy, credentialed model. Before the fix // that combination rerouted the whole request to the substitute model. - credentialsMock = async (model: string) => (model === "kimi-for-coding" ? false : null); + credentialsMock = async (model: string) => (model === BARE_UNKNOWN ? false : null); - const result = await createGuardrail().preCall( - imagePayload("kimi-for-coding", "bare-14003"), - { model: "kimi-for-coding", log: createLogger() } - ); + const result = await createGuardrail().preCall(imagePayload(BARE_UNKNOWN, "bare-14003"), { + model: BARE_UNKNOWN, + log: createLogger(), + }); assert.strictEqual(result.block, false); const body = result.modifiedPayload as Record; assert.strictEqual( body.model, - "kimi-for-coding", + BARE_UNKNOWN, "the explicitly requested model must not be swapped for another provider's model" ); assert.strictEqual( @@ -121,7 +138,7 @@ test("VB-14003-BARE: keeps a bare model id with unknown vision capability instea ); assert.strictEqual(visionCallCount, 1, "images must be described, not dropped"); assert.ok( - warnLines().some((line) => line.includes("kimi-for-coding")), + warnLines().some((line) => line.includes(BARE_UNKNOWN)), "the skipped reroute must name the original model in the log" ); }); @@ -150,18 +167,18 @@ test("VB-14003-QUALIFIED: a provider-qualified id with unknown capability still }); test("VB-14003-QUALIFIED-LEAF: the same wire id under a provider prefix still reroutes", async () => { - // `kimi-for-coding` and `kimi-coding/kimi-for-coding` are the same wire model; + // `deepseek-coder-6.7b` and `llamagate/deepseek-coder-6.7b` are the same wire model; // only the qualified spelling keeps the existing reroute. This is the boundary // the gate must respect. credentialsMock = async () => null; const result = await createGuardrail().preCall( - imagePayload("kimi-coding/kimi-for-coding", "qualified-leaf-14003"), - { model: "kimi-coding/kimi-for-coding", log: createLogger() } + imagePayload(QUALIFIED_UNKNOWN, "qualified-leaf-14003"), + { model: QUALIFIED_UNKNOWN, log: createLogger() } ); const body = result.modifiedPayload as Record; - assert.notStrictEqual(body.model, "kimi-coding/kimi-for-coding"); + assert.notStrictEqual(body.model, QUALIFIED_UNKNOWN); assert.strictEqual((result.meta as Record).rerouted, true); }); @@ -198,4 +215,4 @@ test("VB-14003-AUTO: the unknown-capability gate never fires for an auto/ model" !warnLines().some((line) => line.includes("Vision capability unknown for bare model")), "the unknown-capability skip must never fire for an auto/ model" ); -}); \ No newline at end of file +}); diff --git a/tests/unit/proxy-management-v1-route.test.ts b/tests/unit/proxy-management-v1-route.test.ts index 8dca4bc168e1..c0c9cb60cd08 100644 --- a/tests/unit/proxy-management-v1-route.test.ts +++ b/tests/unit/proxy-management-v1-route.test.ts @@ -152,6 +152,13 @@ test("v1 management proxies main route covers auth, lookup variants, update and assert.equal(deleteAuthRes.status, 401); }); + // The INITIAL_PASSWORD block above persisted setupComplete=true (headless-deploy + // bootstrap in getSettings()). Since #15044, requireManagementAuth no longer inherits + // the public-route shortcut, so a setup-complete install without a password requires + // management auth even from loopback. Start the unauthenticated CRUD branches below + // from a fresh, pre-onboarding install, as this test originally intended. + await resetStorage(); + const providerConn = await providersDb.createProviderConnection({ provider: "openai", authType: "apikey", diff --git a/tests/unit/security/build-profile-stubs.test.ts b/tests/unit/security/build-profile-stubs.test.ts index 710efd0d02ee..8f4d3b4ee5be 100644 --- a/tests/unit/security/build-profile-stubs.test.ts +++ b/tests/unit/security/build-profile-stubs.test.ts @@ -44,6 +44,12 @@ test("cloudSync.stub.ts: syncToCloud soft-fails with feature-disabled message", test("ninerouter.stub.ts: install / resolveSpawnArgs throw FeatureDisabledError", async () => { const stub = await import("../../../src/lib/services/installers/ninerouter.stub.ts"); - await assert.rejects(() => stub.installNinerouter(), /9router-installer/); + // #12810 renamed the stub's `installNinerouter` to `install` (the real module's export + // name) and added the remaining real exports; every one of them must stay disabled. + await assert.rejects(() => stub.install(), /9router-installer/); + await assert.rejects(() => stub.update(), /9router-installer/); + await assert.rejects(() => stub.uninstall(), /9router-installer/); + await assert.rejects(() => stub.getInstalledVersion(), /9router-installer/); + await assert.rejects(() => stub.getLatestVersion(), /9router-installer/); assert.throws(() => stub.resolveSpawnArgs("api-key", 20130), /9router-installer/); });