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

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

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.
Give the bridge a pre-emit barrier for the durable write, or make the close path async.
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.

Corrections and a partial fix. An independent review of this issue found three
factual errors in it plus a missing call site. Recording them here rather than editing
the body silently, since anyone reading from the top would otherwise implement against
the wrong picture.

1. A third call site was missed. The issue names the streaming close path and the
non-streaming pushOutput path. There is a third: failCurrentToolCall in
src/bridge.ts discards the promise the same way at two more sites. It is reached from
six places including the stall watchdog and response.failed, and it carries the same
metadata deliberately so SSE and buffered JSON agree. An implementer working from the
original text would patch two of three.

2. The local claim was backwards. The issue warns that requiring a new field would
make authMode: "local" providers stop remembering. They already never remember:
src/server/responses/core.ts only binds scope.current when credential and
destination identity both exist, and otherwise deletes it — so keyFor already returns
unscoped for them. There is no regression to guard against, and the "state a policy"
note sends the implementer after a non-problem.

3. The version-migration mechanism was mis-stated. version: 2 was written but
never read — load() had no version check at all. Bumping the number would have
invalidated nothing by itself; old entries were simply accepted, went dead against the
new key shape, and aged out on TTL. Silent, and indistinguishable from a store that is
not working.

4. The credential table omits a fourth auth mode. Codex pool auth
(reasoningReplayCodexCredentialIdentity) is built from a rotating bearer plus account
ids — the worst restart-stability story of the four, and absent from the table.

Partial fix landed: ebab9d253 (destination scope only).

The restart-stability blocker binds to the credential component, not to destination.
provider.baseUrl is configuration, stable across restarts by construction; the only
reason today's providerDestinationIdentity is unstable is that it runs through the
random-keyed HMAC. A plain digest of the same normalized URL is equally non-reversible
here and needs no persisted salt. So the cross-endpoint half was self-contained and is
now closed: one provider name serving two endpoints no longer shares signatures.
load() also reads the version now, and the store is at v3 with an explicit drop.

Still open, and these are the more serious ones: cross-account and rotation
collisions, which need a restart-stable credential discriminator per auth mode (OAuth
accountId+generation is usable directly; key auth needs a persisted-salt digest;
Codex pool auth needs a decision about the rotating bearer), and the emit-before-commit
half at all three call sites.

On that second half, one design note the issue should carry: awaiting a disk write per
tool call puts fsync latency on the hot path, which argues for a turn-end barrier rather
than per-item awaits. And the real structural blocker is not that closeCurrentToolCall
is synchronous — the streaming call sites all sit inside an async loop already, so the
obstacle there is the reentrancy guard, not the absence of an await point. It is
buildResponseJSON: a synchronous public export with three callers, which cannot await
without an async signature change rippling outward.

docs(devlog): record what WP4 discovery found, and why it stops there

fix(responses): scope durable thought signatures to the upstream dest…

docs(devlog): adopt close-on-dev-merge for this run only

The destination half landed earlier (ebab9d2, keyFor v3). The remaining credential-scope half is now designed to diff-level: devlog/_plan/260818_bug_pr_resolution/051_tsig_credential_scope.md — restart-stable credential discriminator (preferred: OAuth-subject/apikey digest, matching the destination-digest precedent), keyFor v4 with v3-row drop migration, and a bounded emit-after-commit barrier covering all six bridge sites (streaming close, incomplete-status, buffered flush). This issue closes when both gaps land.

docs(devlog): wp10 follow-up designs + campaign phase records

docs(devlog): close the 260818 bug-PR resolution campaign to _fin

fix(replay): scope durable thought signatures per credential and boun…

Both halves of this issue are now on dev:

Destination scope — landed earlier (keyFor v3, ebab9d2).
Credential scope + commit ordering — fix(replay): scope durable thought signatures per credential and bound persist visibility #2078 (11e03eb): durable keys now carry a salted-HMAC credential identity (persisted OAuth account-slot id / API key / trusted Codex pool handle; fail-closed when none can be derived — no shared credential:unknown slot), STORE_VERSION 4 drops non-upgradable v3 rows, terminal frames await a 250ms-capped durability barrier before becoming externally visible, and the durable lookup is now wired at adapter serialization time (the parse-time lookup could never hit). Independent security review drove three fold-backs (trusted-handle-only bucket selection, salt 0600 re-assertion, runtime wiring regression test).

Residual noted on the PR: the salt lives beside config.json, which today stores API keys in plaintext (#1221) — when keys move to the OS keychain, the salt should follow.

refactor(types): move type clusters to leaves; types.ts becomes a pur…

fix(responses): routed-destination sanitize + opaque-state recovery s…

docs(devlog): record what WP4 discovery found, and why it stops there

fix(responses): scope durable thought signatures to the upstream dest…

docs(devlog): adopt close-on-dev-merge for this run only

docs(devlog): wp10 follow-up designs + campaign phase records

docs(devlog): close the 260818 bug-PR resolution campaign to _fin

fix(replay): scope durable thought signatures per credential and boun…

refactor(types): move type clusters to leaves; types.ts becomes a pur…

fix(responses): routed-destination sanitize + opaque-state recovery s…

Navigation Menu

Repository navigation

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

Client or integration

Provider or upstream service

OpenCodex version

Endpoint or capability

Current behaviour

Expected behaviour

Suggested implementation notes

Additional context

Checks

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

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

Description

Client or integration

Provider or upstream service

OpenCodex version

Endpoint or capability

Current behaviour

Expected behaviour

Suggested implementation notes

Additional context

Checks

Activity

lidge-jun commented on Aug 17, 2026

lidge-jun commented on Aug 18, 2026

lidge-jun commented on Aug 19, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions