mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-31 04:12:10 +03:00
fix(api): persist max_input_tokens/max_output_tokens when adding a custom model (#4349)
The POST /api/provider-models handler read the rest of the body but never the two token-limit fields, and addCustomModel() had no parameter for them, so the form values were dropped on write while the DB layer and /v1/models catalog already round-trip inputTokenLimit/outputTokenLimit. Accept the two optional limits in the schema, forward them through the handler, and persist them in addCustomModel(). TDD: failing-then-passing unit test. Reported-by: codename-zen (https://github.com/decolua/9router/issues/1294) Co-authored-by: codename-zen <263238141+codename-zen@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
2dc38c1618
commit
7ef11eb94e
@@ -24,6 +24,7 @@ _In development — bullets added per PR; finalized at release._
|
||||
- **fix(providers): the "Test Connection One-by-One" OAuth probe can no longer hang the queue forever** — the OAuth connection-test path called bare `fetch(url, { method, headers })` with no `AbortController`/signal/timeout, so when a provider's probe endpoint accepted the socket but never responded, the awaited fetch never settled and the one-by-one test queue stalled indefinitely (the API-key path was already bounded via `validateProviderApiKey`'s `timeoutMs`). Both the initial probe and the post-refresh retry are now bounded with `AbortSignal.timeout(30s)` — matching the API-key path's 30s budget — and a timed-out probe resolves as a failure with a clear `Test timed out after 30s` message in the same shape as every other test error. (thanks @ntdung6868)
|
||||
- **fix(providers): a deactivated account is labeled distinctly from a revoked token** — a Codex connection whose OAuth refresh is fully healthy but whose ChatGPT account has been deactivated by the provider gets a `401` from the upstream API. The connection test labeled that the same as a bad credential (`Token invalid or revoked` → `upstream_auth_error`), so the operator couldn't tell a deactivated account from a revoked token. The test now reads the `401`/`403` body and, when it indicates account deactivation, classifies it as `account_deactivated` — which the dashboard already renders as "Account Deactivated". A plain auth `401` is unchanged. (thanks @ntdung6868)
|
||||
- **fix(db): cascade-delete orphaned model aliases when a provider is removed** — deleting a custom provider removed its connections and node but left behind the imported model-alias rows (stored as `key=<alias>`, `value="<providerId>/<model>"`). Those stale aliases then blocked re-importing the same provider — the import dedup treated them as "already exists", so no new models appeared. A new `deleteModelAliasesForProvider(providerId)` DB helper drops every alias whose stored value begins with `<providerId>/` (leaving other providers and user-defined settings aliases untouched), and the provider-node DELETE handler now calls it after removing the connections and node, so a fresh import is unblocked. (thanks @nguyenvanhuy0612)
|
||||
- **fix(api): persist `max_input_tokens` / `max_output_tokens` when adding a custom model** — `POST /api/provider-models` silently dropped the per-model token limits set in the "add custom model" form: the handler destructured the rest of the body but never read `max_input_tokens` / `max_output_tokens`, and `addCustomModel()` had no parameter for them, so the values were thrown away on write. The DB layer (`inputTokenLimit` / `outputTokenLimit`) and the `/v1/models` catalog already round-trip these fields — only the write path was missing. The validation schema now accepts the two optional limits, the handler forwards them, and `addCustomModel()` persists them so a custom model's context/output window survives into the catalog. (thanks @codename-zen)
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -97,8 +97,18 @@ export async function POST(request) {
|
||||
if (isValidationFailure(validation)) {
|
||||
return Response.json({ error: validation.error }, { status: 400 });
|
||||
}
|
||||
const { provider, modelId, modelName, source, apiFormat, supportedEndpoints, targetFormat } =
|
||||
validation.data;
|
||||
const {
|
||||
provider,
|
||||
modelId,
|
||||
modelName,
|
||||
source,
|
||||
apiFormat,
|
||||
supportedEndpoints,
|
||||
targetFormat,
|
||||
// #1294: persist the per-model token limits set in the add-model form.
|
||||
max_input_tokens: maxInputTokens,
|
||||
max_output_tokens: maxOutputTokens,
|
||||
} = validation.data;
|
||||
|
||||
const model = await addCustomModel(
|
||||
provider,
|
||||
@@ -107,7 +117,11 @@ export async function POST(request) {
|
||||
source || "manual",
|
||||
apiFormat,
|
||||
supportedEndpoints,
|
||||
targetFormat
|
||||
targetFormat,
|
||||
{
|
||||
...(maxInputTokens != null ? { inputTokenLimit: maxInputTokens } : {}),
|
||||
...(maxOutputTokens != null ? { outputTokenLimit: maxOutputTokens } : {}),
|
||||
}
|
||||
);
|
||||
return Response.json({ model });
|
||||
} catch (error) {
|
||||
|
||||
@@ -408,7 +408,10 @@ export async function addCustomModel(
|
||||
// #2905: optional per-model wire format override (e.g. "claude" for an
|
||||
// opencode-go custom model). When unset, routing falls back to the provider
|
||||
// default format.
|
||||
targetFormat?: string
|
||||
targetFormat?: string,
|
||||
// #1294: optional per-model token limits supplied from the "add custom model"
|
||||
// form. Persisted under the same keys the /v1/models catalog reads back.
|
||||
tokenLimits: { inputTokenLimit?: number; outputTokenLimit?: number } = {}
|
||||
) {
|
||||
const db = getDbInstance();
|
||||
const row = db
|
||||
@@ -427,6 +430,12 @@ export async function addCustomModel(
|
||||
apiFormat,
|
||||
supportedEndpoints,
|
||||
...(targetFormat ? { targetFormat } : {}),
|
||||
...(tokenLimits.inputTokenLimit != null
|
||||
? { inputTokenLimit: tokenLimits.inputTokenLimit }
|
||||
: {}),
|
||||
...(tokenLimits.outputTokenLimit != null
|
||||
? { outputTokenLimit: tokenLimits.outputTokenLimit }
|
||||
: {}),
|
||||
};
|
||||
models.push(model);
|
||||
db.prepare(
|
||||
|
||||
@@ -405,6 +405,11 @@ export const providerModelMutationSchema = z.object({
|
||||
targetFormat: z
|
||||
.enum(["openai", "openai-responses", "claude", "gemini", "gemini-cli", "antigravity"])
|
||||
.optional(),
|
||||
// #1294: optional token limits set in the "add custom model" form. The wire
|
||||
// shape uses max_input_tokens / max_output_tokens (mirrors the /v1/models
|
||||
// catalog); they persist as inputTokenLimit / outputTokenLimit.
|
||||
max_input_tokens: z.number().int().positive().optional(),
|
||||
max_output_tokens: z.number().int().positive().optional(),
|
||||
normalizeToolCallId: z.boolean().optional(),
|
||||
preserveOpenAIDeveloperRole: z.boolean().nullable().optional(),
|
||||
upstreamHeaders: upstreamHeadersRecordSchema.nullable().optional(),
|
||||
|
||||
80
tests/unit/provider-models-token-limits.test.ts
Normal file
80
tests/unit/provider-models-token-limits.test.ts
Normal file
@@ -0,0 +1,80 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), "omniroute-provider-model-token-limits-")
|
||||
);
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const modelsDb = await import("../../src/lib/db/models.ts");
|
||||
const providerModelsRoute = await import("../../src/app/api/provider-models/route.ts");
|
||||
|
||||
async function resetStorage() {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
|
||||
}
|
||||
|
||||
function buildPostRequest(body) {
|
||||
return new Request("http://localhost/api/provider-models", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
}
|
||||
|
||||
test.beforeEach(async () => {
|
||||
await resetStorage();
|
||||
});
|
||||
|
||||
test.after(async () => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// #1294: POST /api/provider-models must persist max_input_tokens / max_output_tokens
|
||||
// (stored as inputTokenLimit / outputTokenLimit) so the token limits set in the
|
||||
// "add custom model" form survive into the catalog round-trip.
|
||||
test("POST persists max_input_tokens / max_output_tokens as inputTokenLimit / outputTokenLimit", async () => {
|
||||
const response = await providerModelsRoute.POST(
|
||||
buildPostRequest({
|
||||
provider: "openai-compatible-demo",
|
||||
modelId: "custom-long-context",
|
||||
modelName: "Custom Long Context",
|
||||
max_input_tokens: 200000,
|
||||
max_output_tokens: 16384,
|
||||
})
|
||||
);
|
||||
const body = (await response.json()) as any;
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(body.model?.id, "custom-long-context");
|
||||
assert.equal(body.model?.inputTokenLimit, 200000);
|
||||
assert.equal(body.model?.outputTokenLimit, 16384);
|
||||
|
||||
const stored = await modelsDb.getCustomModels("openai-compatible-demo");
|
||||
const persisted = stored.find((model) => model.id === "custom-long-context");
|
||||
assert.ok(persisted, "custom model should be persisted");
|
||||
assert.equal(persisted.inputTokenLimit, 200000);
|
||||
assert.equal(persisted.outputTokenLimit, 16384);
|
||||
});
|
||||
|
||||
test("POST omits token limits when they are not provided", async () => {
|
||||
const response = await providerModelsRoute.POST(
|
||||
buildPostRequest({
|
||||
provider: "openai-compatible-demo",
|
||||
modelId: "custom-no-limits",
|
||||
modelName: "Custom No Limits",
|
||||
})
|
||||
);
|
||||
const body = (await response.json()) as any;
|
||||
|
||||
assert.equal(response.status, 200);
|
||||
assert.equal(body.model?.id, "custom-no-limits");
|
||||
assert.equal("inputTokenLimit" in body.model, false);
|
||||
assert.equal("outputTokenLimit" in body.model, false);
|
||||
});
|
||||
Reference in New Issue
Block a user