diff --git a/CHANGELOG.md b/CHANGELOG.md index 61f85cf1b7..c20d13ec19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### šŸ› Bug Fixes +- **fix(providers):** GitLab Duo tool-calling follow-up turns no longer fail upstream with `422 {"detail":"Validation error"}` (tokens 0/0, rejected pre-inference). The #6234 tool-result-feedback fix serialized the **entire** multi-turn conversation into GitLab's single-file `code_suggestions` (`small_file`) generation endpoint — folded history that turn-N sent as an oversized `current_file.content_above_cursor` **and** duplicated verbatim into `user_instruction`, tripping the AI-Gateway's `small_file` validation guard. The executor now **bounds** that prompt: it keeps system + latest user message + the most-recent tool round (dropping older turns), caps oversized tool results, and stops duplicating the full prompt into `user_instruction` (which now carries only the short latest user message) — while still feeding the most-recent tool result back so the agent continues (`open-sse/executors/gitlab.ts`, [#6220](https://github.com/diegosouzapw/OmniRoute/issues/6220)). The unit test covers the bounding logic; the upstream 422→200 clearing is VPS-only (Hard Rule #18). Regression guard: `tests/unit/gitlab-tool-exchange-bounded-6220.test.ts`. + - **fix(i18n):** the provider-detail (`/dashboard/providers/[id]`) connection-status filter labels no longer render as `__MISSING__:All` / `__MISSING__:Active` / `__MISSING__:Error` / `__MISSING__:Banned` / `__MISSING__:CreditsExhausted` in non-English locales (notably pt-BR) ([#6290](https://github.com/diegosouzapw/OmniRoute/issues/6290)). Root cause was **not** the namespace mismatch the issue guessed — the `providers.filter*` keys resolve correctly in `en.json`; the debt lived in the locale mirrors (`src/i18n/messages/*.json`), where these five keys carried the `__MISSING__:` sync sentinel in ~15 locales and were absent entirely in ~26 others, so next-intl found the key and echoed the sentinel verbatim. All 40 non-English/-Chinese mirrors now ship real translations for the five `providers.filter*` labels. Regression guard: `tests/unit/i18n-provider-filter-keys-6290.test.ts`. (thanks @diegosouzapw) - **fix(providers):** the `copilot-m365-web` streaming executor now emits `debug`-level WebSocket diagnostics ([#6210](https://github.com/diegosouzapw/OmniRoute/issues/6210)) — the outbound WS URL (with the `access_token` **redacted** via `redactWsUrl()`), handshake success/failure, and each received SignalR frame's `type`/`target`. Previously the streaming path logged nothing, so an empty `content:null` response (the M365 Education / Starter tier symptom fixed in #6234) was undiagnosable even at `APP_LOG_LEVEL=debug`. The change is debug-level and side-effect-free — it does not alter streaming behavior or the frame parser, and the token never reaches the logs. Regression guard: `tests/unit/copilot-m365-web-logging-6210.test.ts` (thanks @qpeyba) diff --git a/open-sse/executors/gitlab.ts b/open-sse/executors/gitlab.ts index 510a8a39e3..594dfa7e47 100644 --- a/open-sse/executors/gitlab.ts +++ b/open-sse/executors/gitlab.ts @@ -43,6 +43,22 @@ type OpenAIMessage = { type JsonRecord = Record; +/** + * Bounds for GitLab's `code_suggestions` `small_file` generation contract. Its AI-Gateway + * validation guard rejects an oversized/over-structured prompt with + * `422 {"detail":"Validation error"}` (tokens 0/0, pre-inference). Turn-1 is small and + * passes; a long folded tool-exchange history trips it — so the serialized prompt (and the + * `user_instruction` field) must stay bounded (#6220). + */ +const MAX_TOOL_EXCHANGE_CHARS = 24_000; +const MAX_TOOL_RESULT_CHARS = 8_000; +const MAX_USER_INSTRUCTION_CHARS = 4_000; + +function capText(text: string, max: number): string { + if (text.length <= max) return text; + return `${text.slice(0, max)}\n…[truncated ${text.length - max} chars]`; +} + type GitLabRequestTarget = { mode: "monolith" | "direct"; url: string; @@ -145,26 +161,76 @@ function renderConversationTurn(message: OpenAIMessage, role: string, text: stri return null; } -/** Serialize the full turn history so the model sees the tool result (#6220). */ +/** + * Keep only what the `small_file` generation contract can carry: every system message, + * the latest user message, and the most-recent tool round (the last assistant `tool_calls` + * turn onward). Older turns are dropped so `content_above_cursor` stays bounded (#6220). + */ +function selectBoundedMessages(messages: OpenAIMessage[]): OpenAIMessage[] { + let lastUserIdx = -1; + let lastToolRoundIdx = -1; + messages.forEach((message, idx) => { + const role = String(message?.role || "user").toLowerCase(); + if (role === "user") lastUserIdx = idx; + if (role === "assistant" && Array.isArray(message?.tool_calls) && message.tool_calls.length) { + lastToolRoundIdx = idx; + } + }); + const tailStart = lastToolRoundIdx >= 0 ? lastToolRoundIdx : lastUserIdx; + const kept: OpenAIMessage[] = []; + messages.forEach((message, idx) => { + const role = String(message?.role || "user").toLowerCase(); + if (role === "system" || role === "developer") { + kept.push(message); + } else if (idx === lastUserIdx || idx >= tailStart) { + kept.push(message); + } + }); + return kept; +} + +/** + * Serialize the most-recent turn history so the model sees the tool result (#6220), but + * BOUNDED: only the last tool round is kept, each tool result is capped, and the whole + * prompt is capped — otherwise the folded history trips GitLab's `small_file` 422 guard. + */ function buildToolExchangePrompt(messages: OpenAIMessage[]): string { const systemParts: string[] = []; const convo: string[] = []; - for (const message of messages) { + for (const message of selectBoundedMessages(messages)) { const role = String(message?.role || "user").toLowerCase(); - const text = extractTextContent(message?.content); + let text = extractTextContent(message?.content); if (role === "system" || role === "developer") { if (text) systemParts.push(text); continue; } + if (role === "tool") text = capText(text, MAX_TOOL_RESULT_CHARS); const line = renderConversationTurn(message, role, text); if (line) convo.push(line); } const header = systemParts.length ? `System instructions:\n${systemParts.join("\n\n")}\n\n` : ""; - return `${header}${convo.join( + const body = `${header}${convo.join( "\n\n" )}\n\nContinue the response using the tool result above; do not repeat the tool call.`.trim(); + return capText(body, MAX_TOOL_EXCHANGE_CHARS); +} + +/** + * The user's actual instruction — the latest user message, capped. Kept separate from the + * folded history so it is NOT duplicated into an oversized `user_instruction`, the likely + * offending 422 field on tool-calling follow-up turns (#6220). + */ +function buildLatestUserInstruction(messages: OpenAIMessage[]): string { + for (let i = messages.length - 1; i >= 0; i--) { + const role = String(messages[i]?.role || "user").toLowerCase(); + if (role === "user") { + const text = extractTextContent(messages[i]?.content); + if (text) return capText(text, MAX_USER_INSTRUCTION_CHARS); + } + } + return ""; } /** @@ -329,7 +395,16 @@ export class GitlabExecutor extends BaseExecutor { _stream: boolean, credentials: ExecuteInput["credentials"] ): Record { - const prompt = buildPrompt(body.messages as OpenAIMessage[] | undefined); + const messages = body.messages as OpenAIMessage[] | undefined; + const prompt = buildPrompt(messages); + // On a tool-exchange follow-up, `content_above_cursor` already carries the (bounded) + // folded history — sending the same long text again as `user_instruction` is the + // likely field that trips GitLab's `small_file` 422 guard. Send only the short latest + // user message there instead (#6220). Simple turns keep the legacy behavior. + const isToolExchange = Array.isArray(messages) && hasToolExchange(messages); + const userInstruction = isToolExchange + ? buildLatestUserInstruction(messages as OpenAIMessage[]) + : prompt; const providerData = credentials?.providerSpecificData && typeof credentials.providerSpecificData === "object" ? credentials.providerSpecificData @@ -354,7 +429,7 @@ export class GitlabExecutor extends BaseExecutor { generation_type: "small_file", stream: false, ...(projectPath ? { project_path: projectPath } : {}), - ...(prompt ? { user_instruction: prompt } : {}), + ...(userInstruction ? { user_instruction: userInstruction } : {}), }; } diff --git a/tests/unit/gitlab-tool-exchange-bounded-6220.test.ts b/tests/unit/gitlab-tool-exchange-bounded-6220.test.ts new file mode 100644 index 0000000000..75c8566aa5 --- /dev/null +++ b/tests/unit/gitlab-tool-exchange-bounded-6220.test.ts @@ -0,0 +1,95 @@ +/** + * Regression test for #6220 (follow-up) — the tool-exchange feedback fix + * (gitlab-tool-result-feedback-6220) taught the GitLab Duo executor to serialize the + * FULL multi-turn conversation into the code_suggestions prompt so the model sees the + * tool result. But GitLab's AI-Gateway `code_suggestions` endpoint is a single-file + * `generation` API with a `small_file` validation guard: once the folded history grew + * large, `transformRequest` sent an oversized `content_above_cursor` AND duplicated the + * whole thing into `user_instruction`, and the gateway rejected turn-N with + * `422 {"detail":"Validation error"}` (tokens 0/0, pre-inference). + * + * Fix: BOUND the serialized tool-exchange prompt — keep system + latest user message + + * the most-recent tool round, cap oversized tool results, and stop duplicating the full + * prompt into `user_instruction` (it now carries only the short latest user message). + * The most-recent tool result must still be present so the model keeps its observation. + * + * The upstream 422→200 clearing is VPS-only (Hard Rule #18); this unit test covers the + * bounding logic (char/length caps + tool-result presence), which is the root cause. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { GitlabExecutor, buildPrompt } from "../../open-sse/executors/gitlab.ts"; + +// A tool result large enough to blow the small_file generation contract if unbounded. +const HUGE_TOOL_RESULT = + "TOOLRESULT_START " + "x".repeat(60_000) + " TOOLRESULT_END"; + +function buildLongToolConversation() { + const messages: Array> = [ + { role: "system", content: "You are a helpful coding assistant." }, + { role: "user", content: "List the files and summarize the repo." }, + ]; + // ≄10 messages: several assistant tool_calls + tool results folded back. + for (let i = 0; i < 5; i++) { + messages.push({ + role: "assistant", + content: "", + tool_calls: [ + { + id: `call_${i}`, + type: "function", + function: { name: "read_file", arguments: `{"path":"file_${i}.ts"}` }, + }, + ], + }); + messages.push({ + role: "tool", + tool_call_id: `call_${i}`, + name: "read_file", + content: i === 4 ? HUGE_TOOL_RESULT : `contents of file_${i}`, + }); + } + return messages; +} + +test("buildPrompt: long tool-exchange history is bounded (char cap) [#6220]", () => { + const messages = buildLongToolConversation(); + const prompt = buildPrompt(messages); + // Sane bound for the small_file generation contract. + assert.ok( + prompt.length < 30_000, + `bounded prompt should stay under 30k chars, got ${prompt.length}` + ); + // The most-recent tool result must still be present (its head survives the cap). + assert.match(prompt, /TOOLRESULT_START/); +}); + +test("transformRequest: content_above_cursor and user_instruction are both bounded [#6220]", () => { + const executor = new GitlabExecutor("gitlab-duo"); + const messages = buildLongToolConversation(); + const out = executor.transformRequest( + "gitlab-duo/model", + { messages }, + false, + {} as never + ) as Record; + + const currentFile = out.current_file as Record; + const contentAbove = String(currentFile.content_above_cursor || ""); + const userInstruction = String(out.user_instruction || ""); + + // content_above_cursor carries the (bounded) folded history + the tool observation. + assert.ok( + contentAbove.length < 30_000, + `content_above_cursor should be bounded, got ${contentAbove.length}` + ); + assert.match(contentAbove, /TOOLRESULT_START/); + + // user_instruction must NOT duplicate the whole huge prompt — the duplication was the + // likely offending 422 field. It carries only the short latest user message. + assert.ok( + userInstruction.length < 5_000, + `user_instruction should be short (not the full prompt), got ${userInstruction.length}` + ); + assert.match(userInstruction, /summarize the repo/); +});