Skip to content

Gemini thought-signature replay omits destination/credential scope and emits tool items before the durable write commits #1926

Description

@lidge-jun

Client or integration

Codex App

Provider or upstream service

Google Gemini / Antigravity (Cloud Code Assist)

OpenCodex version

2.24.2 (present on current dev)

Endpoint or capability

/v1/responses tool-call replay — src/responses/thought-signature-replay.ts

Current behaviour

Two gaps remain in the durable thought-signature store that #1823 introduced. Both are about the scope of a stored signature and the durability of the store, not about the feature working in the happy path.

1. The replay key omits destination and credential identity.

keyFor() derives from five fields: [clientThreadId, providerName, adapterName, modelId, callId].

The sibling in-memory cache src/responses/reasoning-replay-cache.ts uses seven for the same kind of value — it also includes providerDestinationIdentity and credentialIdentity. Both fields exist on the type (src/types.ts) and are populated in src/server/responses/core.ts. So for the durable store, one thread + provider name + model can currently share an opaque signature across:

  • two different upstream endpoints under one provider name,
  • two different accounts,
  • before and after a credential rotation.

Why the fields were left out is a real constraint, not an oversight. The comment in thought-signature-replay.ts says so: the reasoning cache's identities are process-local HMACs (replayIdentityKey = randomBytes(32)), so reusing them verbatim in a durable key would change every key on restart and the store would silently stop working — worse than the current over-broad key, because it would fail closed and invisibly.

A fix therefore needs restart-stable identity material:

Auth mode Material Restart-stable?
OAuth accountId + generation yes, and already non-secret — usable directly
Key derived from provider.apiKey value is stable but is raw secret material; needs a persisted-salt digest
local credentialIdentity is undefined needs an explicit policy — keyFor's guard is all-or-nothing, so a newly-required field would make local providers stop remembering entirely

And because keyFor()'s output is the on-disk key (the snapshot is written with version: 2), widening it invalidates every persisted entry. That needs a version bump with an explicit drop-or-migrate decision; dropping is acceptable, silently mismatching is not.

2. Tool items are emitted before the durable write commits, and a persist failure is swallowed.

rememberThoughtSignatureForReplay() deliberately returns { result, durable } so a caller can await durability before exposing the item — the comment says exactly that. No caller does. In src/bridge.ts the promise is discarded:

void rememberExtraContentForReplay(currentToolCall.callId, currentToolCall.providerMetadata, replayCacheScope);
// ...
...(rememberAndSerializeExtraContent(...).extra ?? {}),   // durable dropped
emit("response.output_item.done", { output_index: currentToolCall.outputIndex, item });

in the streaming close path (both the freeform and function-call branches) and again on the non-streaming pushOutput path.

This one has a structural obstacle worth stating up front: closeCurrentToolCall is a synchronous closure that writes into a ReadableStream controller. There is no await available at that point, so "await the durable promise before emit" is not a one-line change — it needs either an async close path or a pre-emit barrier.

Separately, persist() ends in .catch(() => {}), so a write failure is invisible even to a caller that does await.

Expected behaviour

  • A signature stored under one account/endpoint is never served to another, and a credential rotation does not replay pre-rotation signatures.
  • A signature that was handed to a client is either durably recorded or the failure is observable — not silently best-effort.
  • Restart replay keeps working, which is the whole point of the durable store.

Suggested implementation notes

  1. Derive restart-stable destination and credential identities for this store (not the process-local HMACs), decide the authMode: "local" policy explicitly, and bump the snapshot to version: 3 with a stated drop-or-migrate choice.
  2. Give the bridge a pre-emit barrier for the durable write, or make the close path async.
  3. Replace the swallowed catch with a typed failure that a caller can act on.

Regression coverage should include: cross-account isolation, cross-destination isolation, rotation isolation, same-scoped-key/different-signature fail-closed (already present), write-then-simulated-restart hit (the case a naive fix would break), and a version: 2 file producing no stale-key hit under version: 3.

Additional context

Found while auditing #1823's follow-ups. The conflict detection, byte budget, and thread/provider/adapter/model scoping that #1823 shipped all hold up; these are the two remaining gaps.

Checks

  • I searched existing provider and compatibility issues.
  • The request and response were redacted.
  • The expected behaviour is based on an upstream specification or concrete client requirement.

Metadata

Metadata

Assignees

No one assigned

    Labels

    account-poolOAuth, credentials, Codex pool, quota, failover, plansbugSomething isn't workingproviderProvider adapters, OpenAI-compat presets, upstream API quirksprovider-compatibilityProvider compatibility reports

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions