From f5501cf9a3bc17e14ed88f90ffde40e7a931ba78 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Wed, 16 Sep 2026 06:14:31 -0300 Subject: [PATCH] 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). --- ...13380-gemini-web-system-and-tool-prompt.md | 1 + open-sse/executors/gemini-web.ts | 71 ++++++++++---- .../gemini-web-cookie-rotation-7676.test.ts | 2 +- .../unit/gemini-web-image-retirement.test.ts | 2 +- .../gemini-web-multiturn-context-8371.test.ts | 9 +- .../unit/gemini-web-tool-calling-7286.test.ts | 44 +++++++-- tests/unit/gemini-web.test.ts | 1 + ...ue-13380-gemini-web-system-dropped.test.ts | 93 +++++++++++++++++++ 8 files changed, 192 insertions(+), 31 deletions(-) create mode 100644 changelog.d/fixes/13380-gemini-web-system-and-tool-prompt.md create mode 100644 tests/unit/issue-13380-gemini-web-system-dropped.test.ts diff --git a/changelog.d/fixes/13380-gemini-web-system-and-tool-prompt.md b/changelog.d/fixes/13380-gemini-web-system-and-tool-prompt.md new file mode 100644 index 0000000000..8c1db0a08b --- /dev/null +++ b/changelog.d/fixes/13380-gemini-web-system-and-tool-prompt.md @@ -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 diff --git a/open-sse/executors/gemini-web.ts b/open-sse/executors/gemini-web.ts index b08f84bbee..adbff8329a 100644 --- a/open-sse/executors/gemini-web.ts +++ b/open-sse/executors/gemini-web.ts @@ -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: * @@ -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"); diff --git a/tests/unit/gemini-web-cookie-rotation-7676.test.ts b/tests/unit/gemini-web-cookie-rotation-7676.test.ts index fbc2b46d7a..a6020aa1a2 100644 --- a/tests/unit/gemini-web-cookie-rotation-7676.test.ts +++ b/tests/unit/gemini-web-cookie-rotation-7676.test.ts @@ -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 () => {}, diff --git a/tests/unit/gemini-web-image-retirement.test.ts b/tests/unit/gemini-web-image-retirement.test.ts index c5b3917c07..7e8d63aca2 100644 --- a/tests/unit/gemini-web-image-retirement.test.ts +++ b/tests/unit/gemini-web-image-retirement.test.ts @@ -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 () => {}, diff --git a/tests/unit/gemini-web-multiturn-context-8371.test.ts b/tests/unit/gemini-web-multiturn-context-8371.test.ts index fca4965b9b..46ab90639e 100644 --- a/tests/unit/gemini-web-multiturn-context-8371.test.ts +++ b/tests/unit/gemini-web-multiturn-context-8371.test.ts @@ -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 ─────────────────── diff --git a/tests/unit/gemini-web-tool-calling-7286.test.ts b/tests/unit/gemini-web-tool-calling-7286.test.ts index e7a6ec34ee..2c19860102 100644 --- a/tests/unit/gemini-web-tool-calling-7286.test.ts +++ b/tests/unit/gemini-web-tool-calling-7286.test.ts @@ -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; async function withMockedGeminiBrowser( responseText: string, - fn: (typedPrompt: { value: string }) => Promise + fn: (typedPrompt: { value: string }, calls: string[]) => Promise ): Promise { 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( 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( })) 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 = - '{"name":"get_weather","arguments":{"city":"Berlin"}}'; + const responseText = '{"name":"get_weather","arguments":{"city":"Berlin"}}'; 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"); + }); +}); diff --git a/tests/unit/gemini-web.test.ts b/tests/unit/gemini-web.test.ts index bab367a1c1..67c444b29f 100644 --- a/tests/unit/gemini-web.test.ts +++ b/tests/unit/gemini-web.test.ts @@ -121,6 +121,7 @@ test("Normalizes a bare __Secure-1PSID value before adding browser cookies", asy }), keyboard: { type: async () => {}, + insertText: async () => {}, press: async () => {}, }, }), diff --git a/tests/unit/issue-13380-gemini-web-system-dropped.test.ts b/tests/unit/issue-13380-gemini-web-system-dropped.test.ts new file mode 100644 index 0000000000..e04d9c7fbe --- /dev/null +++ b/tests/unit/issue-13380-gemini-web-system-dropped.test.ts @@ -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"); +});