fix(api): return 400 for missing/invalid messages before model resolution (#6402) (#6515)

fix(api): return 400 for missing/invalid messages before model resolution (#6402). Integrated into release/v3.8.47. (thanks @chirag127)
This commit is contained in:
Chirag Singhal
2026-07-08 04:37:48 +05:30
committed by GitHub
parent 9450e03b1e
commit 3aeb87be7b
3 changed files with 151 additions and 14 deletions

View File

@@ -258,7 +258,7 @@
"src/shared/services/cliRuntime.ts": 1100,
"src/shared/validation/schemas.ts": 2523,
"_rebaseline_2026_06_28_5275_correlation_id_extract": "Extraction of the safe CorrelationId subset of #5275 (hartmark) — request correlation id stored in call_logs (migration 109) and returned via the X-Correlation-Id response header, WITHOUT the combo/resilience or build/lazy-loading changes (those stay in #5275). Own growth: callLogs.ts 975->985 (correlation_id column on CallLogSummaryRow + read/map), usageHistory.ts 983->988 (correlationId metadata normalize), chat.ts 1575->1632 (withCorrelationId response wiring + combo-failure log carrying correlationId), chatHelpers.ts new 811 (withCorrelationId helper + reqId threading; was 791<cap pre-feature). Cohesive request/logging chokepoint wiring; structural shrink of chat.ts tracked in #3501.",
"src/sse/handlers/chat.ts": 1751,
"src/sse/handlers/chat.ts": 1763,
"src/sse/handlers/chatHelpers.ts": 866,
"src/sse/services/auth.ts": 2448,
"open-sse/executors/default.ts": 877,
@@ -383,5 +383,6 @@
"_rebaseline_2026_07_06_6351_glm_team_quota": "PR #6351 own growth (GLM team-plan quota fields threaded through the connection modals; new GlmTeamQuotaFields.tsx extracted): AddApiKeyModal.tsx ->951 (+9), EditConnectionModal.tsx ->1277 (+18). Absorbs the pre-existing session base-red on these frozen modals; release captain rebaseline-at-release supersedes.",
"_rebaseline_2026_07_06_6499_unique_default_name": "PR #6499 own growth: AddApiKeyModal.tsx 952->959 (+7 = a unique default connection name so a second API key for the same provider does not reuse 'main' and trigger the backend name-based upsert that silently overwrote the first connection). The pure name derivation was extracted to computeConnectionDefaultName.ts (unit-tested) to keep the growth minimal; the contributor's original full-form-reset rewrite was trimmed to a spread reset to avoid dropping the GLM team-quota fields #6351 added and to hold the frozen god-file growth down. Release captain rebaseline-at-release supersedes.",
"_rebaseline_2026_07_07_6523_chirag_cooldown_body": "PR #6523 (@chirag127, #6460) own growth: chatHelpers.ts 860->866 (+6 = retryAfterAt/credentialsCoolingCount fields on modelCooldownResponse) and auth.ts 2447->2448 (+1 = connectionsCount threaded through no-credentials fallback). Owner-approved rebaseline (file-size cap for contributor PR). Frozen (cannot grow further); release captain's rebaseline-at-release supersedes.",
"_rebaseline_2026_07_07_6526_chirag_modal_1080p": "PR #6526 (@chirag127, #6265): AddApiKeyModal.tsx ->961 (1080p sizing). Owner-approved. Frozen."
"_rebaseline_2026_07_07_6526_chirag_modal_1080p": "PR #6526 (@chirag127, #6265): AddApiKeyModal.tsx ->961 (1080p sizing). Owner-approved. Frozen.",
"_rebaseline_2026_07_07_6515_chirag": "PR #6515 (@chirag127) own growth: src/sse/handlers/chat.ts ->1763. Owner-approved rebaseline. Frozen."
}

View File

@@ -240,18 +240,30 @@ export async function handleChat(
// reasoning_effort / reasoning / object-shaped thinking always wins (backward compatible).
body = normalizeReasoningRequest(body);
// Early guard: an explicitly empty `messages` array is invalid for every
// upstream (Anthropic/OpenAI both reject "at least one message is required").
// Forwarding it produced a confusing raw upstream 400/502; reject it here with
// a clear OmniRoute-level error before any routing or upstream call (#5110).
// Responses-API requests use `input` (not `messages`) so they are unaffected,
// and an absent `messages` field is left to downstream validation.
if (
Array.isArray((body as { messages?: unknown }).messages) &&
(body as { messages: unknown[] }).messages.length === 0
) {
log.warn("CHAT", "Rejecting request with empty messages array");
return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required");
// Early guard: an invalid `messages` field is rejected here with a clear
// OmniRoute-level 400 before any routing or upstream call (#5110, #6402).
// Without this guard, schema-invalid bodies fell through to model resolution
// and surfaced as a misleading 404 `model_not_found` from chatHelpers.ts (#6402).
// Cases covered:
// - present-but-non-array (null, number, string, object) → 400 (#6402)
// - empty array → 400 ("at least one message is required") (#5110)
// - missing entirely, when the Responses-API `input` discriminator is also
// absent → 400 (#6402). Responses-API requests use `input` (not `messages`)
// and are still unaffected.
{
const b = body as { messages?: unknown; input?: unknown };
if ("messages" in b && !Array.isArray(b.messages)) {
log.warn("CHAT", "Rejecting request with non-array messages");
return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array");
}
if (Array.isArray(b.messages) && b.messages.length === 0) {
log.warn("CHAT", "Rejecting request with empty messages array");
return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: at least one message is required");
}
if (!("messages" in b) && !("input" in b)) {
log.warn("CHAT", "Rejecting request with missing messages");
return errorResponse(HTTP_STATUS.BAD_REQUEST, "messages: Expected array, received undefined");
}
}
// Reject non-string `model` before it reaches downstream code that calls

View File

@@ -0,0 +1,124 @@
import test from "node:test";
import assert from "node:assert/strict";
import { createChatPipelineHarness } from "../integration/_chatPipelineHarness.ts";
// Regression tests for #6402 — schema-invalid `messages` fields fell through
// the existing empty-array guard and reached model resolution, where an
// unresolvable model surfaced as a misleading 404 `model_not_found` from
// chatHelpers.ts (`No active credentials for provider: <p>`).
//
// The guard at src/sse/handlers/chat.ts now rejects three additional cases with
// a clear OmniRoute-level 400 before any routing or upstream call:
// - present-but-null messages
// - present-but-non-array messages (number, string, object)
// - missing messages when the Responses-API `input` discriminator is also
// absent.
const harness = await createChatPipelineHarness("chat-messages-validation-6402");
const { handleChat, buildRequest, resetStorage, seedConnection } = harness;
test.beforeEach(async () => {
await resetStorage();
});
test.after(async () => {
await harness.cleanup();
});
test("#6402: messages: null is rejected with a clear 400 before model resolution", async () => {
await seedConnection("anthropic", { apiKey: "sk-ant" });
let upstreamCalled = false;
globalThis.fetch = async () => {
upstreamCalled = true;
return new Response("{}", { status: 200, headers: { "content-type": "application/json" } });
};
const response = await handleChat(
buildRequest({
body: {
model: "anthropic/claude-haiku-4-5",
messages: null,
},
})
);
assert.equal(response.status, 400, "null messages must be a 400, not a 404 model_not_found");
const body = (await response.json()) as { error?: { message?: string } };
assert.match(body.error?.message ?? "", /messages.*Expected array/i);
assert.equal(upstreamCalled, false, "must not forward a null-messages request upstream");
});
test("#6402: messages: <number> is rejected with a clear 400 before model resolution", async () => {
await seedConnection("anthropic", { apiKey: "sk-ant" });
let upstreamCalled = false;
globalThis.fetch = async () => {
upstreamCalled = true;
return new Response("{}", { status: 200, headers: { "content-type": "application/json" } });
};
const response = await handleChat(
buildRequest({
body: {
model: "anthropic/claude-haiku-4-5",
messages: 123 as unknown as [],
},
})
);
assert.equal(response.status, 400, "non-array messages must be a 400, not a 404 model_not_found");
const body = (await response.json()) as { error?: { message?: string } };
assert.match(body.error?.message ?? "", /messages.*Expected array/i);
assert.equal(upstreamCalled, false, "must not forward a non-array-messages request upstream");
});
test("#6402: missing messages (and no Responses-API input) is rejected with a clear 400", async () => {
await seedConnection("anthropic", { apiKey: "sk-ant" });
let upstreamCalled = false;
globalThis.fetch = async () => {
upstreamCalled = true;
return new Response("{}", { status: 200, headers: { "content-type": "application/json" } });
};
const response = await handleChat(
buildRequest({
body: {
model: "anthropic/claude-haiku-4-5",
},
})
);
assert.equal(response.status, 400, "missing messages must be a 400, not a 404 model_not_found");
const body = (await response.json()) as { error?: { message?: string } };
assert.match(body.error?.message ?? "", /messages.*Expected array/i);
assert.equal(upstreamCalled, false, "must not forward a missing-messages request upstream");
});
test("#6402: Responses-API input passes the guard (messages discriminator preserved)", async () => {
await seedConnection("openai", { apiKey: "sk-openai" });
const response = await handleChat(
buildRequest({
url: "http://localhost/v1/responses",
body: {
model: "openai/gpt-4.1",
input: [{ role: "user", content: "Hello" }],
},
})
);
// The guard must not fire for Responses-API requests; whatever downstream
// status this produces (200, 404, etc.), it MUST NOT be the guard's
// "messages: Expected array" 400.
if (response.status === 400) {
const body = (await response.json()) as { error?: { message?: string } };
assert.doesNotMatch(
body.error?.message ?? "",
/messages.*Expected array/i,
"Responses-API request must not be caught by the messages guard"
);
}
});