Skip to content

fix(translator): scope thoughtSignature bypass to Antigravity/CLI only (supersedes #3414) - #3514

Closed
oyi77 wants to merge 26 commits into
cheaper-inference:release/v3.8.20from
oyi77:fix/gemini-400-signature-and-array-errors
Closed

oyi77 wants to merge 26 commits into
cheaper-inference:release/v3.8.20from
oyi77:fix/gemini-400-signature-and-array-errors

Conversation

@oyi77

@oyi77 oyi77 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Problem

PR #3414 introduced a skip_thought_signature_validator bypass sentinel injected via openaiToGeminiBase, which is shared between standard Gemini, Gemini CLI, and Antigravity. As @gemini-code-assist correctly identified:

  1. HIGH: Standard Gemini API (FORMATS.GEMINI) does NOT support the thoughtSignature field — it rejects requests with 400
  2. The bypass was unconditionally injected for all callers, breaking standard Gemini tool calls

Fix

Added supportsSignatureBypass option to GeminiToolNameOptions:

  • Set to true only in openaiToGeminiCLIRequest (which serves Gemini CLI + Antigravity)
  • Standard Gemini callers (openaiToGeminiRequest) leave it undefined → no bypass injected

Changes

  • open-sse/translator/request/openai-to-gemini.ts: Added supportsSignatureBypass option, conditional bypass injection
  • tests/unit/t43-gemini-tool-call-no-thought-signature.test.ts: Reverted test expectations for standard Gemini (should NOT get bypass)

Tests

  • All 39 translator tests pass (t43, t44, translator-openai-to-gemini)
  • Standard Gemini: no thoughtSignature injected ✅
  • Antigravity: bypass sentinel correctly preserved ✅

Supersedes #3414 (includes all original fixes + addresses review comments)

diegosouzapw and others added 16 commits June 9, 2026 08:31
…aper-inference#3481)

Codex's model-catalog refresh (codex_models_manager) does
GET /v1/models?client_version=<v> and decodes a JSON object with a
TOP-LEVEL `models` array. OmniRoute answers in the OpenAI-standard
`{object,data}` shape, so codex fails with "missing field `models`"
and logs "failed to refresh available models" on every startup.

Detect codex clients via the `originator` / `user-agent` = `codex_*`
headers they send and add an EMPTY top-level `models: []` so the decode
succeeds. Non-codex OpenAI clients keep the byte-identical `{object,data}`
response.

The array is intentionally empty: codex replaces its built-in per-model
agent prompt (`base_instructions`, ~21k chars) with whatever a populated
entry carries for the selected model, so emitting our catalog would drop
the agent prompt to nothing and break codex's agent behaviour (verified
empirically against codex 0.137). An empty list keeps codex on its
built-in model info — same inference as before, minus the error.

Validated end-to-end with the real handler against codex 0.137:
"failed to refresh available models" → 0 occurrences, instructions
preserved (built-in Codex agent prompt, not empty).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add generated quality gate reports, metrics files, and local setup prompt
artifacts to .gitignore to prevent committing environment-specific or
temporary files.
…g retry text (cheaper-inference#3513)

PR cheaper-inference#3422 ('allow OpenAI usage-only empty choices chunks') reintroduced the
assistant-content injection '[OmniRoute] Upstream returned an empty response.
Please retry.' for empty `choices: []` chunks that carry no valid usage. Clients
(Goose/opencode) feed that text back as a turn and spin in a retry loop -- the
exact regression cheaper-inference#3400 had fixed by dropping the chunk.

Restore the drop behavior for the no-usage case while preserving cheaper-inference#3422's
standards-compliant forwarding of usage-only `include_usage` final chunks.
Realign the mislabeled stream-utils test (it asserted the injection) and add a
dedicated regression guard.

Reported-by: @mochizzan
Refs: cheaper-inference#3502, cheaper-inference#3388, cheaper-inference#3400, cheaper-inference#3422

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a signature bypass mechanism ("skip_thought_signature_validator") for Antigravity/Gemini models to prevent 400 errors on signature-less tool calls, refactoring the request translator to emit native function calls and responses instead of legacy XML/text-based formatting. It also adds error parsing support for array responses and updates the unit tests. A critical issue was identified in openai-to-gemini.ts where a redundant check causes duplicate IDs to be pushed to toolCallIds, resulting in duplicated tool responses in the generated payload.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines 408 to 410
if (!signatureForToolCall && contextualizeSignaturelessToolResponses) {
if (!toolCallIds.includes(id)) toolCallIds.push(id);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since the legacy signature-less text/context mode continue statements and custom formatting have been removed, all tool calls that pass the initial checks will unconditionally reach line 433 and have their id pushed to toolCallIds.

Keeping this block causes duplicate IDs to be pushed to toolCallIds when !signatureForToolCall && contextualizeSignaturelessToolResponses is true, which in turn duplicates the tool responses in the generated payload. We should remove this redundant block entirely.

@oyi77
oyi77 force-pushed the fix/gemini-400-signature-and-array-errors branch from cc0dc4a to 3be18d1 Compare June 9, 2026 17:29
…polynomial-ReDoS)

CodeQL js/polynomial-redos: unbounded \s* before an anchored \s*$ allowed
O(n²) backtracking on attacker-influenced display names. Bounded to {0,8}/{1,8}
(ample for any real label spacing). Plugin builds + 254 tests green.
diegosouzapw and others added 6 commits June 9, 2026 15:01
- getPendingRequests() typed to real shape (was widened to object) → fixes
  unknown 'count' in the unified-requests view (cheaper-inference#3401)
- streamChunks log payload cast to its declared type (callLogs.ts)
- preScreenTargets aligned to canonical IsModelAvailable signature (cheaper-inference#3169),
  Promise.resolve-normalized so .catch never hits a bare boolean

All 5 gates green: lint(0 err) + typecheck:core + cycles + docs-all + unit + vitest(146).
Standard Gemini API (FORMATS.GEMINI) does not support the thoughtSignature
field and rejects requests containing it with 400. The bypass sentinel
'skip_thought_signature_validator' was being unconditionally injected by
openaiToGeminiBase, which is shared between standard Gemini, Gemini CLI,
and Antigravity.

Fix: add supportsSignatureBypass option to GeminiToolNameOptions, set it
true only in openaiToGeminiCLIRequest (which serves both CLI and Antigravity).
Standard Gemini callers (openaiToGeminiRequest) leave it undefined/false.

Addresses review comment from @gemini-code-assist on PR cheaper-inference#3414.
The conditional toolCallIds.push(id) was redundant since the unconditional
push at the end of the loop always executes. This caused duplicate tool
response entries when contextualizeSignaturelessToolResponses was true.

Addresses HIGH priority review comment from @gemini-code-assist on PR cheaper-inference#3514.
@oyi77
oyi77 force-pushed the fix/gemini-400-signature-and-array-errors branch from 1aca0cd to d3978be Compare June 9, 2026 18:06
@oyi77
oyi77 changed the base branch from main to release/v3.8.18 June 9, 2026 18:41
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.18 to release/v3.8.19 June 9, 2026 19:46
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.19 to release/v3.8.20 June 10, 2026 06:15
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @oyi77. The intended fix here (thoughtSignature bypass) is a real problem worth solving, but I can't merge this PR as-is: it touches 136 files for what should be a ~6-file change. ~42 of them are docs/i18n/*/CHANGELOG.md + root CHANGELOG.md translation dumps, and dozens more (combo.ts, browserPool.ts, chatCore.ts, package.json×3, etc.) carry unrelated incidental edits from a stale rebase. A reviewer can't safely sign off on a translator fix buried in 130 files of noise.

This PR is also CONFLICTING. The clean, correctly-scoped base for this fix is #3414 (6 files) — I've left detailed review notes there (the bypass needs to be scoped to the Antigravity/CLI call site rather than applied to all non-text Gemini traffic, client-provided signatures shouldn't be overwritten, and the #3358 contextualize logic shouldn't be silently dropped).

Could you land the fix on a clean branch off current release/v3.8.20 with only the translator/executor/error files (no i18n/CHANGELOG dump)? That's mergeable; this isn't. Leaving open so your work isn't lost. 🙏

@oyi77

oyi77 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #3560 (focused 6-file re-cut off release/v3.8.20 per @diegosouzapw review). Closing this one in favor of the cleaner PR.

@oyi77 oyi77 closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants