fix(chat): stop forwarding redundant provider-node routing segments to model lookup (#11557)

Validated in a combined 4-PR batch worktree off release/v3.8.51 tip.
- Focused test: model-connid-prefix-normalization-6772.test.ts — part of batch's 60/60 node:test run
- typecheck:core, file-size, changelog-integrity, complexity, cognitive-complexity, check:docs-counts-sync — all OK
- Full-repo lint: 228 pre-existing dashboard react-hooks/* findings, unrelated to this diff

Thanks for closing this with production-log-shaped test cases — a deterministic upstream 404 loop from a duplicated routing segment is exactly the kind of defect that's easy to miss without real traffic shapes in the test suite.
This commit is contained in:
Dizzle
2026-08-26 00:48:49 +02:00
committed by GitHub
parent ffd8984a59
commit 3863dab149
3 changed files with 131 additions and 18 deletions

View File

@@ -0,0 +1 @@
- fix(chat): shed any of the matched provider-node's routing identifiers (public prefix or internal id) before model lookup, so composites like `<connId>/<connId>/<model>` no longer reach upstream verbatim (#11557)

View File

@@ -388,17 +388,34 @@ async function lookupModelMeta(
/**
* When a custom provider node is matched by its raw internal `node.id` (e.g. a combo
* step addressing `<connId>/...` — see #2778), `parsed.model` was never split on the
* node's own `prefix`, unlike the alias-addressing path where `parseModel` already
* strips it. If the caller naively concatenates `owned_by` (the node's prefix, as
* listed by /api/models) with the raw model id, the resulting model string carries a
* redundant leading `${node.prefix}/` segment that the upstream provider does not
* recognize, causing a 400. Strip it so `<connId>/<prefix>/<rawModelId>` normalizes to
* the same `<rawModelId>` the bare alias form resolves to (#6772).
* node's own identifiers, unlike the alias-addressing path where `parseModel` already
* strips the prefix. If the caller naively concatenates routing segments with the raw
* model id, the resulting model string carries redundant leading segments that the
* upstream provider does not recognize, causing deterministic 404s (retried).
*
* Observed in production traffic: `<connId>/<connId>/<model>` — requests addressed
* by the node's internal id (#2778) left a second `<connId>/` segment in parsed.model
* that the historical strip (prefix alone, #6772) never saw, and the composite went
* upstream verbatim. We now shed ANY of the matched node's routing identifiers (prefix AND internal id), repeatedly, until stable.
* A legitimate namespace different from these identifiers is untouched (#493);
* an operator naming their prefix identically to one of their catalog namespaces
* sees that namespace shed — accepted limitation, precedent #6772.
*/
function stripRedundantNodePrefix(model: string, nodePrefix: unknown): string {
if (typeof nodePrefix !== "string" || !nodePrefix) return model;
const redundant = `${nodePrefix}/`;
return model.startsWith(redundant) ? model.slice(redundant.length) : model;
function stripRedundantNodeRoutingSegments(model: string, routingIds: unknown[]): string {
let out = model;
let changed = true;
while (changed) {
changed = false;
for (const seg of routingIds) {
if (typeof seg !== "string" || !seg) continue;
const redundant = `${seg}/`;
if (out.startsWith(redundant)) {
out = out.slice(redundant.length);
changed = true;
}
}
}
return out;
}
/**
@@ -457,10 +474,10 @@ export async function getModelInfo(modelStr) {
(node) => node.prefix === prefixToCheck || node.id === prefixToCheck
);
if (matchedOpenAI) {
const normalizedModel = stripRedundantNodePrefix(
parsed.model as string,
matchedOpenAI.prefix
);
const normalizedModel = stripRedundantNodeRoutingSegments(parsed.model as string, [
matchedOpenAI.prefix,
matchedOpenAI.id,
]);
const { modelId, metadata } = await lookupModelMeta(
matchedOpenAI.id as string,
normalizedModel
@@ -479,10 +496,10 @@ export async function getModelInfo(modelStr) {
(node) => node.prefix === prefixToCheck || node.id === prefixToCheck
);
if (matchedAnthropic) {
const normalizedModel = stripRedundantNodePrefix(
parsed.model as string,
matchedAnthropic.prefix
);
const normalizedModel = stripRedundantNodeRoutingSegments(parsed.model as string, [
matchedAnthropic.prefix,
matchedAnthropic.id,
]);
const { modelId, metadata } = await lookupModelMeta(
matchedAnthropic.id as string,
normalizedModel

View File

@@ -84,3 +84,98 @@ test("#6772 RED: `<connId>/<prefix>/<rawModelId>` (naive owned_by+id concat) mus
`"${RAW_MODEL_ID}" — got "${info.model}" instead (double-namespaced, will 400 upstream)`
);
});
// ── shedding redundant routing segments (node prefix AND internal id) ──
const AC_CONN_ID = "anthropic-compatible-a1111111-probeshed";
const AC_PREFIX = "acustpfx";
test.before(async () => {
await providersDb.createProviderNode({
id: AC_CONN_ID,
type: "anthropic-compatible",
name: "probe shed",
prefix: AC_PREFIX,
baseUrl: "https://proxy.example.com",
chatPath: "/v1/messages",
modelsPath: "/v1/models",
});
await modelsDb.addCustomModel(
AC_CONN_ID,
RAW_MODEL_ID,
"vova gpt-5.5",
"manual",
"chat-completions",
["chat"]
);
});
test("shed measured production-log shape: `<connId>/<connId>/<bare>` sheds to `<bare>`", async () => {
const info = (await getModelInfo(`${CONN_ID}/${CONN_ID}/gpt-oss-20b`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, "gpt-oss-20b", `got "${info.model}"`);
});
test("shed full logged composite: double connId + namespace keeps the namespace after shed", async () => {
// Real production log shape: `<connId>/<connId>/openai/gpt-oss-20b:free`.
// Shedding removes the matched node's OWN identifiers only; the `openai/`
// namespace is out of scope — this test pins that exact promise.
const info = (await getModelInfo(`${CONN_ID}/${CONN_ID}/openai/gpt-oss-20b:free`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, "openai/gpt-oss-20b:free", `got "${info.model}"`);
});
test("shed mixed addressing: `<prefix>/<connId>/<raw>` sheds to `<raw>`", async () => {
const info = (await getModelInfo(`${PREFIX}/${CONN_ID}/${RAW_MODEL_ID}`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, RAW_MODEL_ID, `got "${info.model}"`);
});
test("shed #493 guard: legitimate namespace distinct from the node's identifiers stays intact", async () => {
const info = (await getModelInfo(`${CONN_ID}/zai-org/GLM-5-FP8`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, "zai-org/GLM-5-FP8", `got "${info.model}"`);
});
test("shed SYNTHETIC triple stack: `<connId>/<prefix>/<connId>/<raw>` sheds to `<raw>`", async () => {
const info = (await getModelInfo(`${CONN_ID}/${PREFIX}/${CONN_ID}/${RAW_MODEL_ID}`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, RAW_MODEL_ID, `got "${info.model}"`);
});
test("shed accepted limitation (#6772 precedent): operator prefix equal to a catalog namespace is shed", async () => {
// An operator naming their prefix like a real upstream namespace sees that
// namespace shed — indistinguishable without querying the catalog.
// DOCUMENTARY LOCK: same input as the "bare alias form" baseline above — no
// new behavioral coverage; pins the accepted limitation as shedding, not refusal.
const info = (await getModelInfo(`${PREFIX}/${RAW_MODEL_ID}`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, CONN_ID);
assert.equal(info.model, RAW_MODEL_ID);
});
test("shed anthropic-compatible parity: `<acConnId>/<acPrefix>/<raw>` sheds to `<raw>`", async () => {
const info = (await getModelInfo(`${AC_CONN_ID}/${AC_PREFIX}/${RAW_MODEL_ID}`)) as {
provider?: string;
model?: string;
};
assert.equal(info.provider, AC_CONN_ID);
assert.equal(info.model, RAW_MODEL_ID, `got "${info.model}"`);
});