diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec6aedb60..acbf704144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ ### 🔧 Bug Fixes +- **fix(github):** drop a trailing assistant prefill before dispatching to GitHub Copilot chat to avoid 400 errors. (thanks @baslr) + - **providers (Kiro — Claude Sonnet 5):** the Kiro provider's model catalog was missing `claude-sonnet-5`, so the model could not be selected or routed even on accounts that already had access to it ("claude-sonnet-5 is not supported"). Added the model to the Kiro registry (`open-sse/config/providers/registry/kiro/index.ts`) as a 1M-context / 128K-output Claude model, mirroring the existing Claude entries; the registry `models[]` feeds both the model selector and the live CodeWhisperer `ListAvailableModels` fallback, so the model is now selectable and routable. Regression guard: `tests/unit/kiro-claude-sonnet-5-2267.test.ts`. (thanks [@openbioinfo](https://github.com/openbioinfo)) - **settings (model aliases — self-heal after restart):** the Settings → Routing page showed "No exact-match aliases configured" after a server restart even though the aliases were persisted in the DB. Aliases are held in a module-local `_customAliases` map in `modelDeprecation.ts` that the boot path hydrates, but Next.js compiles the app-route module graph separately from the startup graph (the same webpack chunk-splitting class as #5312), so the `GET /api/settings/model-aliases` handler read a different, un-hydrated copy. The handler now self-heals: when its in-memory alias map is empty it reads `settings.modelAliases` from the DB (via the existing `getSettings()` db module — no raw SQL in the route) and repopulates the map, so the UI reflects the persisted aliases on the first GET after a restart. Follow-up: the root cause is now also fixed — the `_customAliases` store in `modelDeprecation.ts` is backed by `globalThis` (key `__omniroute_customAliases__`), so the startup and app-route module graphs share **one** store and the route reads the boot-hydrated aliases directly (the DB self-heal remains as a harmless fallback), mirroring the same `globalThis` singleton pattern already applied to `thinkingBudget.ts`/`backgroundTaskDetector.ts` (#5312). Regression guards: `tests/unit/model-aliases-settings-route-selfheal.test.ts` + `tests/unit/model-aliases-globalthis-5777.test.ts`. ([#5777](https://github.com/diegosouzapw/OmniRoute/pull/5777) — thanks [@jleonar2](https://github.com/jleonar2)) diff --git a/open-sse/executors/github.ts b/open-sse/executors/github.ts index f2507bd9cf..5271500d31 100644 --- a/open-sse/executors/github.ts +++ b/open-sse/executors/github.ts @@ -149,6 +149,18 @@ export class GithubExecutor extends BaseExecutor { ); } + // GitHub Copilot's /chat/completions endpoint rejects a conversation that ends + // with an assistant message: "This model does not support assistant message + // prefill. The conversation must end with a user message." (HTTP 400). Anthropic + // clients such as newest Claude Desktop send a trailing assistant turn as a + // prefill seed — the Anthropic API honors it, but Copilot does not. Drop it here, + // scoped to the GitHub executor only (the shared translator/contextManager and + // other providers that DO honor prefill are untouched). + // Port of 9router#2143 (author: Manuel ). + if (Array.isArray(modifiedBody.messages)) { + modifiedBody.messages = this.dropTrailingAssistantPrefill(modifiedBody.messages); + } + // Config-driven strip of params unsupported by the target provider/model. // For GitHub Copilot this removes Claude-style `thinking` and // `reasoning_effort` for Claude models that reject them upstream @@ -188,6 +200,19 @@ export class GithubExecutor extends BaseExecutor { return { ...msg, content: cleanContent.length > 0 ? cleanContent : null }; } + // Remove trailing assistant message(s). GitHub Copilot's /chat/completions endpoint + // can't honor an assistant prefill and 400s unless the conversation ends with a + // non-assistant (user/tool) message. Never empties the array — an assistant-only + // conversation keeps its last message. No-op (same array reference) when the + // conversation already ends with a non-assistant message. + // Port of 9router#2143 (author: Manuel ). + dropTrailingAssistantPrefill(messages: any): any { + if (!Array.isArray(messages) || messages.length === 0) return messages; + let end = messages.length; + while (end > 1 && messages[end - 1]?.role === "assistant") end--; + return end === messages.length ? messages : messages.slice(0, end); + } + async execute(input: ExecuteInput) { const result = await super.execute(input); if (!result || !result.response) return result; diff --git a/tests/unit/executor-github-prefill-sanitize.test.ts b/tests/unit/executor-github-prefill-sanitize.test.ts new file mode 100644 index 0000000000..346882951b --- /dev/null +++ b/tests/unit/executor-github-prefill-sanitize.test.ts @@ -0,0 +1,119 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { GithubExecutor } from "../../open-sse/executors/github.ts"; + +// GitHub Copilot's /chat/completions endpoint rejects a conversation that ends with an +// assistant message: "This model does not support assistant message prefill. The +// conversation must end with a user message." Anthropic clients (e.g. newest Claude +// Desktop) send a trailing assistant turn as a prefill seed — the Anthropic API honors +// it, but Copilot 400s. GithubExecutor.dropTrailingAssistantPrefill() strips the +// trailing assistant message(s) before dispatch, scoped to the github executor only. +// Port of 9router#2143 (author: Manuel ). + +test("dropTrailingAssistantPrefill drops a single trailing assistant message", () => { + const executor = new GithubExecutor(); + const messages = [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "Here is the answer:" }, + ]; + + const out = executor.dropTrailingAssistantPrefill(messages); + + assert.equal(out.length, 1); + assert.equal(out[0].role, "user"); +}); + +test("dropTrailingAssistantPrefill drops multiple consecutive trailing assistant messages", () => { + const executor = new GithubExecutor(); + const messages = [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "one" }, + { role: "assistant", content: "two" }, + ]; + + const out = executor.dropTrailingAssistantPrefill(messages); + + assert.equal(out.length, 1); + assert.equal(out[0].role, "user"); +}); + +test("dropTrailingAssistantPrefill is a no-op (same reference) when the conversation ends with a user message", () => { + const executor = new GithubExecutor(); + const messages = [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "Hello" }, + { role: "user", content: "More" }, + ]; + + const out = executor.dropTrailingAssistantPrefill(messages); + + assert.equal(out, messages, "must return the same array reference when nothing changes"); + assert.equal(out.length, 3); +}); + +test("dropTrailingAssistantPrefill is a no-op when the conversation ends with a tool message", () => { + const executor = new GithubExecutor(); + const messages = [ + { role: "user", content: "Hi" }, + { role: "assistant", content: null, tool_calls: [{ id: "x" }] }, + { role: "tool", tool_call_id: "x", content: "result" }, + ]; + + const out = executor.dropTrailingAssistantPrefill(messages); + + assert.equal(out, messages, "must return the same array reference when nothing changes"); + assert.equal(out.length, 3); + assert.equal(out[2].role, "tool"); +}); + +test("dropTrailingAssistantPrefill never empties an assistant-only conversation", () => { + const executor = new GithubExecutor(); + const messages = [{ role: "assistant", content: "only" }]; + + const out = executor.dropTrailingAssistantPrefill(messages); + + assert.equal(out.length, 1, "must keep at least one message"); + assert.equal(out[0].role, "assistant"); +}); + +test("dropTrailingAssistantPrefill is null/empty safe", () => { + const executor = new GithubExecutor(); + + assert.deepEqual(executor.dropTrailingAssistantPrefill([]), []); + assert.equal(executor.dropTrailingAssistantPrefill(undefined), undefined); + assert.equal(executor.dropTrailingAssistantPrefill(null), null); +}); + +test("GithubExecutor.transformRequest drops the trailing assistant prefill end-to-end", () => { + const executor = new GithubExecutor(); + const body = { + model: "claude-sonnet-4.6", + messages: [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "Here is the answer:" }, + ], + }; + + const out = executor.transformRequest("claude-sonnet-4.6", body, false, {}); + + assert.equal(out.messages.length, 1); + assert.equal(out.messages[0].role, "user"); +}); + +test("GithubExecutor.transformRequest leaves a user-terminated conversation untouched end-to-end", () => { + const executor = new GithubExecutor(); + const body = { + model: "claude-sonnet-4.6", + messages: [ + { role: "user", content: "Hi" }, + { role: "assistant", content: "Hello" }, + { role: "user", content: "More" }, + ], + }; + + const out = executor.transformRequest("claude-sonnet-4.6", body, false, {}); + + assert.equal(out.messages.length, 3); + assert.equal(out.messages[2].role, "user"); +}); diff --git a/tests/unit/executor-github.test.ts b/tests/unit/executor-github.test.ts index a4ae4eb4d3..cf4efb0006 100644 --- a/tests/unit/executor-github.test.ts +++ b/tests/unit/executor-github.test.ts @@ -112,6 +112,11 @@ test("GithubExecutor.transformRequest injects JSON response instructions for Cla reasoning_text: "internal", reasoning_content: "internal", }, + // Trailing user turn: dropTrailingAssistantPrefill (9router#2143) strips a + // conversation that ends in "assistant", which would otherwise remove the very + // message this test inspects below. Keep the array ending in "user" so this test + // stays focused on response_format injection + reasoning-field stripping. + { role: "user", content: "thanks" }, ], }; @@ -225,6 +230,11 @@ test("GithubExecutor.transformRequest leaves string content and missing content role: "assistant", tool_calls: [{ id: "c1", type: "function", function: { name: "f", arguments: "{}" } }], }, + // Trailing tool response: dropTrailingAssistantPrefill (9router#2143) strips a + // conversation that ends in "assistant", which would otherwise remove the very + // tool_calls message this test inspects below. A real tool round-trip ends in + // "tool", not "assistant" — model that shape instead. + { role: "tool", tool_call_id: "c1", content: "result" }, ], }; const result = executor.transformRequest("claude-sonnet-4.6", body, true, {});