mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-17 04:12:17 +03:00
fix(providers): gemini-web no longer drops system instructions or the tool contract (#13380) (#13784)
Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).
This commit is contained in:
committed by
GitHub
parent
57f41f5dd1
commit
f5501cf9a3
@@ -0,0 +1 @@
|
||||
- **fix(providers):** gemini-web no longer drops the system instruction on single-turn requests or the tool contract when a client system message is present, and switches to an atomic composer insert so embedded newlines can't submit the message early (#13380) — thanks @formilw
|
||||
@@ -96,9 +96,13 @@ function formatStreamChunk(content: string, model: string, finishReason: string
|
||||
* flatten the full history into one prompt so the web UI still sees the
|
||||
* conversation.
|
||||
*
|
||||
* Single-turn requests are preserved byte-for-byte (only the final user message
|
||||
* is returned) — the regression guard for the pre-existing no-tools path.
|
||||
* Multi-turn requests emit a labeled transcript:
|
||||
* Single-turn requests with NO system message are preserved byte-for-byte
|
||||
* (only the final user message is returned) — the regression guard for the
|
||||
* pre-existing no-tools path. A single-turn request that DOES carry a system
|
||||
* message prepends it the same way the multi-turn branch does (#13380 — the
|
||||
* old fast path silently dropped the system instruction whenever there was
|
||||
* no prior user/assistant turn, e.g. title generation or a one-shot chat
|
||||
* completion). Multi-turn requests emit a labeled transcript:
|
||||
*
|
||||
* System:
|
||||
* <system text>
|
||||
@@ -125,16 +129,19 @@ export function buildGeminiPrompt(messages: Array<{ role: string; content: unkno
|
||||
(m, i) => i < lastUserIdx && (m.role === "user" || m.role === "assistant")
|
||||
);
|
||||
|
||||
// Single-turn (no earlier user/assistant turns): byte-for-byte the original
|
||||
// single-message derivation. Do NOT prepend system text here — the old
|
||||
// no-tools path ignored a system-only prefix on the first turn.
|
||||
if (priorTurns.length === 0) return lastUserContent;
|
||||
|
||||
const systemText = textMessages
|
||||
.filter((m) => m.role === "system")
|
||||
.map((m) => m.content)
|
||||
.join("\n\n");
|
||||
|
||||
// Single-turn (no earlier user/assistant turns) with no system message:
|
||||
// byte-for-byte the original single-message derivation.
|
||||
if (priorTurns.length === 0 && !systemText) return lastUserContent;
|
||||
|
||||
// Single-turn with a system message (#13380): prepend it instead of
|
||||
// silently dropping it.
|
||||
if (priorTurns.length === 0) return `System:\n${systemText}\n\n${lastUserContent}`;
|
||||
|
||||
const historyLines = priorTurns.map(
|
||||
(m) => `${m.role === "assistant" ? "Assistant" : "User"}: ${m.content}`
|
||||
);
|
||||
@@ -148,18 +155,29 @@ export function buildGeminiPrompt(messages: Array<{ role: string; content: unkno
|
||||
|
||||
/**
|
||||
* Build the plain-text prompt typed into the Gemini web UI when a tool
|
||||
* contract is active — the synthetic system message injected by
|
||||
* `prepareToolMessages()` prepended to the last user message. gemini-web
|
||||
* only ever sends a single flat string (no native message array), so the
|
||||
* tool contract and the user's ask are concatenated (#7286).
|
||||
* contract is active — every system message (the client's own instruction(s)
|
||||
* plus the synthetic tool contract that `prepareToolMessages()` appends last)
|
||||
* prepended, in order, to the last user message. gemini-web only ever sends
|
||||
* a single flat string (no native message array), so the tool contract and
|
||||
* the user's ask are concatenated (#7286).
|
||||
*
|
||||
* `prepareToolMessages()` (open-sse/translator/webTools.ts) pushes the
|
||||
* synthetic tool contract as the LAST system message so it never buries a
|
||||
* long client system prompt. Picking only the FIRST system message
|
||||
* (`.find()`) therefore dropped the tool contract whenever the client
|
||||
* already sent its own system message — #13380. Joining ALL system messages
|
||||
* in order keeps the client instruction(s) first and the tool contract last,
|
||||
* matching that dual-placement design intent.
|
||||
*/
|
||||
export function buildGeminiToolPrompt(
|
||||
effectiveMessages: Array<{ role: string; content: unknown }>
|
||||
): string {
|
||||
const toolSystemMsg = effectiveMessages.find((m) => m.role === "system");
|
||||
const toolPrompt = effectiveMessages
|
||||
.filter((m) => m.role === "system" && typeof m.content === "string")
|
||||
.map((m) => m.content as string)
|
||||
.join("\n\n");
|
||||
const lastUserMsg = [...effectiveMessages].reverse().find((m) => m.role === "user");
|
||||
const userText = typeof lastUserMsg?.content === "string" ? lastUserMsg.content : "";
|
||||
const toolPrompt = typeof toolSystemMsg?.content === "string" ? toolSystemMsg.content : "";
|
||||
return toolPrompt ? `${toolPrompt}\n\n${userText}` : userText;
|
||||
}
|
||||
|
||||
@@ -456,13 +474,23 @@ export class GeminiWebExecutor extends BaseExecutor {
|
||||
// hasTools === false: flatten the full multi-turn history into the single
|
||||
// prompt so gemini-web (a stateless web-cookie provider that captures only
|
||||
// the first StreamGenerate response) preserves prior context across turns
|
||||
// (#8371). Single-turn requests stay byte-for-byte identical to the original
|
||||
// derivation, keeping the #7286 no-tools regression guard intact.
|
||||
// (#8371). Single-turn requests with no system message stay byte-for-byte
|
||||
// identical to the original derivation; a single-turn system message is
|
||||
// now prepended instead of silently dropped (#13380).
|
||||
const prompt = hasTools
|
||||
? buildGeminiToolPrompt(effectiveMessages)
|
||||
: buildGeminiPrompt(messages);
|
||||
|
||||
if (!prompt) {
|
||||
// A system-only request (no user message at all) must still 400 — since
|
||||
// #13380 prepends the system text, `prompt` alone is no longer a
|
||||
// reliable "no user message" signal for the no-tools path (it used to be
|
||||
// empty for a system-only request; now it carries the system text).
|
||||
const hasUserMessage = messages.some(
|
||||
(m: { role: string; content: unknown }) =>
|
||||
m.role === "user" && typeof m.content === "string" && m.content.trim().length > 0
|
||||
);
|
||||
|
||||
if (!prompt || (!hasTools && !hasUserMessage)) {
|
||||
return {
|
||||
response: new Response(JSON.stringify({ error: "No user message found" }), {
|
||||
status: 400,
|
||||
@@ -535,7 +563,14 @@ export class GeminiWebExecutor extends BaseExecutor {
|
||||
timeout: 10000,
|
||||
});
|
||||
await inputEl.click();
|
||||
await page.keyboard.type(prompt, { delay: 10 });
|
||||
// insertText() dispatches a DOM `input` event atomically instead of a
|
||||
// per-character keydown/keypress/keyup sequence (#13380) — an embedded
|
||||
// `\n` in `prompt` (produced by the multi-turn transcript format above,
|
||||
// or by any multiline system/user text) no longer fires the
|
||||
// composer's Enter-submits-the-message handler before this function's
|
||||
// own explicit Enter below. It also removes the fixed 10ms/char typing
|
||||
// cost that made long prompts race the 30s response-wait timeout.
|
||||
await page.keyboard.insertText(prompt);
|
||||
await page.waitForTimeout(300);
|
||||
await page.keyboard.press("Enter");
|
||||
|
||||
|
||||
@@ -53,7 +53,7 @@ test("#7676: GeminiWebExecutor persists rotated __Secure-1PSIDTS/__Secure-1PSIDC
|
||||
goto: async () => {},
|
||||
waitForTimeout: async () => {},
|
||||
waitForSelector: async () => ({ click: async () => {} }),
|
||||
keyboard: { type: async () => {}, press: async () => {} },
|
||||
keyboard: { type: async () => {}, insertText: async () => {}, press: async () => {} },
|
||||
}),
|
||||
}),
|
||||
close: async () => {},
|
||||
|
||||
@@ -68,7 +68,7 @@ test("Gemini Web executor treats the retired image-mode extension as ordinary ch
|
||||
waitDurations.push(duration);
|
||||
},
|
||||
waitForSelector: async () => ({ click: async () => {} }),
|
||||
keyboard: { type: async () => {}, press: async () => {} },
|
||||
keyboard: { type: async () => {}, insertText: async () => {}, press: async () => {} },
|
||||
}),
|
||||
}),
|
||||
close: async () => {},
|
||||
|
||||
@@ -19,14 +19,15 @@ test("#8371: single user message returns that message verbatim (single-turn unch
|
||||
assert.equal(prompt, "What about Paris?");
|
||||
});
|
||||
|
||||
test("#8371: single user turn with a system message still returns only the user text", () => {
|
||||
// Preserves the pre-existing no-tools derivation, which ignored system-only
|
||||
// context on the first turn.
|
||||
test("#13380: single user turn with a system message prepends the system text instead of dropping it", () => {
|
||||
// The pre-existing no-tools derivation ignored system-only context on the
|
||||
// first turn, silently dropping it (#13380). Fixed to prepend it the same
|
||||
// way the multi-turn branch below does.
|
||||
const prompt = buildGeminiPrompt([
|
||||
{ role: "system", content: "You are helpful" },
|
||||
{ role: "user", content: "Hello" },
|
||||
]);
|
||||
assert.equal(prompt, "Hello");
|
||||
assert.equal(prompt, "System:\nYou are helpful\n\nHello");
|
||||
});
|
||||
|
||||
// ─── Multi-turn: full history is flattened into the prompt ───────────────────
|
||||
|
||||
@@ -11,9 +11,8 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { GeminiWebExecutor, buildGeminiToolResponse, buildGeminiToolPrompt } = await import(
|
||||
"../../open-sse/executors/gemini-web.ts"
|
||||
);
|
||||
const { GeminiWebExecutor, buildGeminiToolResponse, buildGeminiToolPrompt } =
|
||||
await import("../../open-sse/executors/gemini-web.ts");
|
||||
|
||||
interface ToolCallLike {
|
||||
function: { name: string; arguments: string };
|
||||
@@ -192,11 +191,12 @@ type FakeResponseHandler = (resp: FakePlaywrightResponse) => Promise<void>;
|
||||
|
||||
async function withMockedGeminiBrowser<T>(
|
||||
responseText: string,
|
||||
fn: (typedPrompt: { value: string }) => Promise<T>
|
||||
fn: (typedPrompt: { value: string }, calls: string[]) => Promise<T>
|
||||
): Promise<T> {
|
||||
const playwright = await import("playwright");
|
||||
const originalLaunch = playwright.chromium.launch;
|
||||
const typedPrompt = { value: "" };
|
||||
const calls: string[] = [];
|
||||
|
||||
playwright.chromium.launch = (async () => ({
|
||||
newContext: async () => ({
|
||||
@@ -212,9 +212,15 @@ async function withMockedGeminiBrowser<T>(
|
||||
waitForSelector: async () => ({ click: async () => {} }),
|
||||
keyboard: {
|
||||
type: async (text: string) => {
|
||||
calls.push("type");
|
||||
typedPrompt.value = text;
|
||||
},
|
||||
insertText: async (text: string) => {
|
||||
calls.push("insertText");
|
||||
typedPrompt.value = text;
|
||||
},
|
||||
press: async () => {
|
||||
calls.push("press");
|
||||
if (respHandler) {
|
||||
await respHandler({
|
||||
url: () => "https://gemini.google.com/_/BardChatUi/data/.../StreamGenerate?x",
|
||||
@@ -231,15 +237,14 @@ async function withMockedGeminiBrowser<T>(
|
||||
})) as unknown as typeof playwright.chromium.launch;
|
||||
|
||||
try {
|
||||
return await fn(typedPrompt);
|
||||
return await fn(typedPrompt, calls);
|
||||
} finally {
|
||||
playwright.chromium.launch = originalLaunch;
|
||||
}
|
||||
}
|
||||
|
||||
test("#7286: executor integration — tools[] present reaches tool_calls end to end", async () => {
|
||||
const responseText =
|
||||
'<tool>{"name":"get_weather","arguments":{"city":"Berlin"}}</tool>';
|
||||
const responseText = '<tool>{"name":"get_weather","arguments":{"city":"Berlin"}}</tool>';
|
||||
|
||||
await withMockedGeminiBrowser(responseText, async () => {
|
||||
const executor = new GeminiWebExecutor();
|
||||
@@ -291,3 +296,28 @@ test("#7286: no-tool passthrough regression — unchanged prompt derivation + re
|
||||
assert.equal(choice.finish_reason, "stop");
|
||||
});
|
||||
});
|
||||
|
||||
test("#13380: the Playwright input uses an atomic insertText, not the per-keystroke type()", async () => {
|
||||
await withMockedGeminiBrowser("ok", async (typedPrompt, calls) => {
|
||||
const executor = new GeminiWebExecutor();
|
||||
const result = await executor.execute({
|
||||
model: "gemini-3.1-pro",
|
||||
body: {
|
||||
messages: [{ role: "user", content: "multi\nline\nprompt" }],
|
||||
stream: false,
|
||||
},
|
||||
stream: false,
|
||||
credentials: { apiKey: "test-cookie" },
|
||||
signal: AbortSignal.timeout(10000),
|
||||
log: null,
|
||||
});
|
||||
|
||||
assert.equal(result.response.status, 200);
|
||||
assert.equal(typedPrompt.value, "multi\nline\nprompt");
|
||||
// insertText dispatches one atomic `input` event instead of per-character
|
||||
// keydown/keypress/keyup, so an embedded "\n" cannot fire the composer's
|
||||
// Enter-submits handler ahead of the executor's own explicit Enter below.
|
||||
assert.ok(!calls.includes("type"), "must not use the per-keystroke type() input path");
|
||||
assert.deepEqual(calls, ["insertText", "press"], "insertText once, then Enter exactly once");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -121,6 +121,7 @@ test("Normalizes a bare __Secure-1PSID value before adding browser cookies", asy
|
||||
}),
|
||||
keyboard: {
|
||||
type: async () => {},
|
||||
insertText: async () => {},
|
||||
press: async () => {},
|
||||
},
|
||||
}),
|
||||
|
||||
93
tests/unit/issue-13380-gemini-web-system-dropped.test.ts
Normal file
93
tests/unit/issue-13380-gemini-web-system-dropped.test.ts
Normal file
@@ -0,0 +1,93 @@
|
||||
// #13380 — gemini-web drops system instructions on single-turn requests, and
|
||||
// buildGeminiToolPrompt() picks the CLIENT's first system message instead of
|
||||
// the appended tool contract when tools are active.
|
||||
//
|
||||
// Bug 1: buildGeminiPrompt()'s single-turn fast path
|
||||
// (open-sse/executors/gemini-web.ts) returned only the last user message,
|
||||
// silently dropping any system instruction when there was no prior
|
||||
// user/assistant turn (title generation, structured extraction, one-shot
|
||||
// chat completions).
|
||||
//
|
||||
// Bug 2: buildGeminiToolPrompt() used
|
||||
// `effectiveMessages.find(m => m.role === "system")`, which returns the
|
||||
// FIRST system message. `prepareToolMessages()` (open-sse/translator/
|
||||
// webTools.ts) appends the synthetic tool contract as the LAST system
|
||||
// message, so any request that already carries a client system message
|
||||
// (any real agent request) lost the tool contract entirely.
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
const { buildGeminiPrompt, buildGeminiToolPrompt } =
|
||||
await import("../../open-sse/executors/gemini-web.ts");
|
||||
const { prepareToolMessages } = await import("../../open-sse/translator/webTools.ts");
|
||||
|
||||
test("#13380 bug 1: single-turn system + user request retains BOTH contents", () => {
|
||||
const messages = [
|
||||
{ role: "system", content: "SYSTEM_SENTINEL\nSECOND_SYSTEM_LINE" },
|
||||
{ role: "user", content: "USER_SENTINEL" },
|
||||
];
|
||||
const prompt = buildGeminiPrompt(messages);
|
||||
assert.ok(prompt.includes("USER_SENTINEL"));
|
||||
assert.ok(prompt.includes("SYSTEM_SENTINEL"));
|
||||
});
|
||||
|
||||
test("#13380 bug 1: single-turn request with no system message stays byte-for-byte identical", () => {
|
||||
const messages = [{ role: "user", content: "JUST_THE_USER_MESSAGE" }];
|
||||
const prompt = buildGeminiPrompt(messages);
|
||||
assert.equal(prompt, "JUST_THE_USER_MESSAGE");
|
||||
});
|
||||
|
||||
test("#13380 bug 2: tool-enabled request retains the appended tool contract, not just the client's first system message", () => {
|
||||
const bodyObj = {
|
||||
tools: [
|
||||
{
|
||||
type: "function",
|
||||
function: {
|
||||
name: "ping",
|
||||
description: "Return a ping",
|
||||
parameters: { type: "object", properties: {} },
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const messages = [
|
||||
{ role: "system", content: "CLIENT_SYSTEM_SENTINEL" },
|
||||
{ role: "user", content: "USER_SENTINEL" },
|
||||
];
|
||||
|
||||
const { effectiveMessages } = prepareToolMessages(bodyObj, messages);
|
||||
const systemMessages = effectiveMessages.filter((m: { role: string }) => m.role === "system");
|
||||
assert.ok(systemMessages.length >= 2);
|
||||
|
||||
const prompt = buildGeminiToolPrompt(effectiveMessages);
|
||||
assert.ok(prompt.includes("Return a ping"));
|
||||
});
|
||||
|
||||
test("#13380 bug 2: tool-enabled request preserves order — client system message(s) before the appended tool contract", () => {
|
||||
const bodyObj = {
|
||||
tools: [
|
||||
{
|
||||
type: "function",
|
||||
function: {
|
||||
name: "ping",
|
||||
description: "Return a ping",
|
||||
parameters: { type: "object", properties: {} },
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
const messages = [
|
||||
{ role: "system", content: "CLIENT_SYSTEM_LINE_1\nCLIENT_SYSTEM_LINE_2" },
|
||||
{ role: "user", content: "USER_SENTINEL" },
|
||||
];
|
||||
|
||||
const { effectiveMessages } = prepareToolMessages(bodyObj, messages);
|
||||
const prompt = buildGeminiToolPrompt(effectiveMessages);
|
||||
|
||||
const clientIdx = prompt.indexOf("CLIENT_SYSTEM_LINE_1");
|
||||
const contractIdx = prompt.indexOf("Return a ping");
|
||||
assert.ok(clientIdx !== -1, "client system message must be present");
|
||||
assert.ok(contractIdx !== -1, "tool contract must be present");
|
||||
assert.ok(clientIdx < contractIdx, "client system message must come before the tool contract");
|
||||
});
|
||||
Reference in New Issue
Block a user