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

devlog/_plan/260817_wave5_execution/040_thought_signature_scope.md

@@ -103,3 +103,77 @@ implementation, not left to the guard's default.
 File the issue with these conditions, implement, close citing the merge SHA and the
 isolation + restart test output. If the discovery step shows no live caller, the
 issue records that finding instead of claiming a fix.
+## Outcome (executed) — discovery done, implementation deferred with reasons
+
+**The caller-discovery step resolved the open question, and the answer changes the
+shape of the work.** The round-1 audit could find no `src/` caller of the remember
+API and concluded the emit-before-commit defect might not be live. It is live. The
+callers are in `src/bridge.ts`, and they discard the durable promise explicitly:
+
+```ts
+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 (freeform and function-call branches) and again on the
+non-streaming `pushOutput` path.
+
+**Why this is not a one-line fix.** `closeCurrentToolCall` is a *synchronous* closure
+writing into a `ReadableStream` controller. There is no `await` at that point, so
+"await durability before emit" requires either an async close path or a pre-emit
+barrier — a change to the streaming core, which `AGENTS.md` gates behind the full
+suite and which sits next to the subagent-fallback synchrony invariant documented in
+the repository root. That is its own work-phase, not a rider on a scope fix.
+
+**Filed as #1926** with both halves, the restart-stability constraint, the
+OAuth/key/local credential-identity split, the `version: 2` → `version: 3` migration
+requirement, and the exact caller locations. Terminal outcome for this cycle:
+**NEEDS_HUMAN on sequencing** — the fix is well-specified and the constraint that
+blocked the naive version is written down, but landing it means touching the
+streaming emit path, which deserves a maintainer's call on scheduling rather than an
+agent slipping it into a wave.
+## Amendment after the WP4 audit — the destination half landed
+
+The reviewer pushed back on deferring the whole key-scope fix, and was right. The
+restart-stability blocker binds to the **credential** component only:
+`provider.baseUrl` is configuration, stable across restarts by construction, and 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. This document already sanctioned that fallback — *"scope the fix
+to destination only"* — and then deferred the branch it had pre-authorized.
+
+Landed in `ebab9d253`:
+
+- `durableReplayDestinationIdentity()` beside the process-local form (not replacing it —
+  the in-memory cache is right to prefer the random-keyed version).
+- `providerDestinationDurableIdentity` threaded through the identity type and `core.ts`.
+- The durable `keyFor` includes it, closing cross-endpoint collisions under one provider name.
+- `load()` reads the store version for the first time; `STORE_VERSION = 3` drops v2
+  entries explicitly instead of letting them go dead and age out invisibly.
+
+Ablation: removing the destination component fails the new cross-endpoint test while the
+restart test stays green — the pair that matters, since the naive fix would have passed
+the first and broken the second.
+
+### Corrections to this document and to #1926
+
+The audit found four errors in what I wrote, all corrected on the issue rather than
+edited away:
+
+1. **A third call site.** `failCurrentToolCall` discards the durable promise too, at two
+   more sites, reached from six places including the stall watchdog. The writeup named
+   only `closeCurrentToolCall` and `flushToolCall`.
+2. **The `local` claim was backwards.** Those providers already never remember — `core.ts`
+   only binds `scope.current` when credential *and* destination exist. There was no
+   regression to protect against.
+3. **The version mechanism was mis-stated.** `version: 2` was written and never read, so a
+   bump would have invalidated nothing. Fixed by actually reading it.
+4. **A fourth auth mode.** Codex pool auth rides a rotating bearer — the worst
+   restart-stability story of the four, and absent from the table.
+
+And one correction to the deferral reasoning itself: the streaming call sites are already
+inside an `async` loop, so the obstacle there is the reentrancy guard, not the absence of
+an await point. The real blocker is `buildResponseJSON`, a synchronous public export with
+three callers. Awaiting a disk write per tool call would also put fsync latency on the hot
+path, which argues for a turn-end barrier rather than per-item awaits.

src/responses/reasoning-replay-cache.ts

@@ -19,7 +19,7 @@
  * long-lived proxy cannot grow without limit.
  */
 
-import { createHmac, randomBytes } from "node:crypto";
+import { createHash, createHmac, randomBytes } from "node:crypto";
 import type {
   OcxProviderConfig,
   OcxReasoningReplayIdentity,
@@ -108,6 +108,21 @@ export function reasoningReplayDestinationIdentity(baseUrl: string | undefined):
   return `destination:${processLocalIdentity("destination", canonical)}`;
 }
 
+/**
+ * The same destination identity, but stable across restarts.
+ *
+ * The process-local form above is keyed by `randomBytes(32)` minted at module load, which
+ * is correct for an in-memory cache and fatal for a durable one: every key would change on
+ * restart and the store would silently stop matching anything. A plain digest of the same
+ * canonical URL is equally non-reversible for this purpose — the input is a configured
+ * endpoint, not a secret — and needs no persisted salt or new on-disk state.
+ */
+export function durableReplayDestinationIdentity(baseUrl: string | undefined): string | undefined {
+  if (!nonEmpty(baseUrl)) return undefined;
+  const canonical = baseUrl.trim().replace(/\/+$/, "");
+  return `destination:${createHash("sha256").update("destination\0").update(canonical).digest("hex")}`;
+}
+
 /** Produce a non-reversible process-local identity for credential material. */
 export function reasoningReplayCredentialIdentity(
   kind: "key" | "oauth" | "codex",

src/responses/thought-signature-replay.ts

@@ -27,6 +27,11 @@ import type { OcxProviderOpaqueToolCallMetadata, OcxReasoningReplayScopeRef } fr
 import { isCarryableSignature, responsesExtraContentFromProviderMetadata } from "./provider-opaque-metadata";
 
 const STORE_FILE_NAME = "thought-signature-replay.json";
+/**
+ * Bumped whenever `keyFor` changes shape. v3 added the durable destination identity, so a
+ * v2 file's keys can never match and are dropped on load instead of aging out invisibly.
+ */
+const STORE_VERSION = 3;
 
 /** Bound on remembered entries; real signatures are a few hundred bytes, so this stays small. */
 const MAX_ENTRIES = 16_384;
@@ -83,6 +88,11 @@ function keyFor(callId: string, scope: OcxReasoningReplayScopeRef | undefined):
   return JSON.stringify([
     scope.clientThreadId,
     identity.providerName,
+    // Destination, unlike the credential identity, has a restart-stable form: it is a
+    // configured endpoint rather than a secret, so a plain digest works where the
+    // reasoning cache's randomBytes-keyed HMAC cannot. Without it, one provider NAME
+    // serving two endpoints shares signatures across both.
+    identity.providerDestinationDurableIdentity ?? "destination:unknown",
     identity.adapterName,
     identity.modelId,
     callId,
@@ -103,6 +113,12 @@ function load(): void {
     if (typeof parsed !== "object" || parsed === null || !Array.isArray((parsed as { entries?: unknown }).entries)) {
       return;
     }
+    // The version was written but never read, so a key-shape change could not be
+    // announced — old entries simply went dead and aged out on TTL, which is silent and
+    // indistinguishable from a store that is not working. Reading it makes a shape change
+    // an explicit drop: entries keyed by an older scheme are discarded on load rather than
+    // lingering as permanent misses.
+    if ((parsed as { version?: unknown }).version !== STORE_VERSION) return;
     const nowMs = Date.now();
     for (const entry of (parsed as { entries: unknown[] }).entries) {
       if (typeof entry !== "object" || entry === null) continue;
@@ -141,7 +157,7 @@ function persist(): Promise<void> {
   persistChain = persistChain
     .then(async () => {
       const snapshot = JSON.stringify({
-        version: 2,
+        version: STORE_VERSION,
         entries: [...entries].map(([key, entry]) => ({ key, sig: entry.sig, savedAt: entry.savedAt })),
       });
       await atomicWriteFileAsync(storePath(), snapshot);

src/server/responses/core.ts

@@ -14,6 +14,7 @@ import {
   bindReasoningReplayScope,
   reasoningReplayCodexCredentialIdentity,
   reasoningReplayDestinationIdentity,
+  durableReplayDestinationIdentity,
   reasoningReplayKeyCredentialIdentity,
   reasoningReplayOAuthCredentialIdentity,
 } from "../../responses/reasoning-replay-cache";
@@ -336,6 +337,7 @@ function bindRouteReasoningReplayScope(args: {
       ? {
           providerName,
           providerDestinationIdentity,
+          providerDestinationDurableIdentity: durableReplayDestinationIdentity(provider.baseUrl),
           adapterName,
           modelId: parsed.modelId,
           credentialIdentity,

src/types.ts

@@ -5,6 +5,11 @@ export interface OcxReasoningReplayIdentity {
   providerName: string;
   /** Opaque process-local digest of the exact upstream destination. */
   providerDestinationIdentity: string;
+  /**
+   * The same destination, digested WITHOUT the process-local random key, so it can key a
+   * durable store. Absent when no base URL was resolvable.
+   */
+  providerDestinationDurableIdentity?: string;
   adapterName: string;
   modelId: string;
   /** Opaque process-local credential identity; never a raw token or API key. */

tests/google-signature-history-roundtrip.test.ts

@@ -15,6 +15,7 @@ import {
   rememberThoughtSignatureForReplay,
   resetThoughtSignatureReplayForTests,
 } from "../src/responses/thought-signature-replay";
+import { durableReplayDestinationIdentity } from "../src/responses/reasoning-replay-cache";
 import type { AdapterEvent, OcxParsedRequest, OcxProviderConfig } from "../src/types";
 import { withTestTranslatorBudget } from "./helpers/translator-budget";
 
@@ -38,12 +39,18 @@ const provider = {
  * client-visible call_id is not unique across threads, accounts, providers or models,
  * so keying on it alone let one conversation's signature reach another's turn.
  */
-function scopeFor(threadId = "thread-a", modelId = MODEL, providerName = "google") {
+function scopeFor(
+  threadId = "thread-a",
+  modelId = MODEL,
+  providerName = "google",
+  destination = "https://generativelanguage.googleapis.com",
+) {
   return {
     clientThreadId: threadId,
     current: {
       providerName,
       providerDestinationIdentity: `dest-${providerName}`,
+      providerDestinationDurableIdentity: durableReplayDestinationIdentity(destination),
       adapterName: "google",
       modelId,
       credentialIdentity: `cred-${providerName}`,
@@ -298,4 +305,34 @@ describe("#1735 thought signature survives history replay", () => {
     resetThoughtSignatureReplayForTests();
     expect(lookupReplayThoughtSignature("call_disk_1", scopeFor())).toBe(SIGNATURE);
   });
+
+  test("one provider name serving two endpoints does not share signatures", () => {
+    // The gap the durable key closes. providerName, adapterName, modelId and thread can all
+    // be identical across two upstreams — a gateway and a direct endpoint under one config
+    // name — and an opaque signature minted by one is meaningless to the other.
+    const primary = scopeFor("thread-a", MODEL, "google", "https://generativelanguage.googleapis.com");
+    const secondary = scopeFor("thread-a", MODEL, "google", "https://gateway.internal.example/v1beta");
+
+    rememberThoughtSignatureForReplay("call_dest", SIGNATURE, primary);
+
+    expect(lookupReplayThoughtSignature("call_dest", primary)).toBe(SIGNATURE);
+    expect(lookupReplayThoughtSignature("call_dest", secondary)).toBeUndefined();
+  });
+
+  test("the durable destination identity is stable across restarts, unlike the process-local one", async () => {
+    // The reason this is a separate digest rather than the sibling cache's HMAC: that one is
+    // keyed by randomBytes minted at module load, so reusing it here would change every key
+    // on restart and the store would silently stop matching — a worse failure than the
+    // over-broad key it replaced, because it looks like it is working.
+    const url = "https://generativelanguage.googleapis.com";
+    expect(durableReplayDestinationIdentity(url)).toBe(durableReplayDestinationIdentity(url));
+    expect(durableReplayDestinationIdentity(url)).not.toBe(durableReplayDestinationIdentity("https://other.example"));
+    // Trailing-slash normalization matches the process-local form.
+    expect(durableReplayDestinationIdentity(`${url}/`)).toBe(durableReplayDestinationIdentity(url));
+
+    rememberThoughtSignatureForReplay("call_dest_restart", SIGNATURE, scopeFor());
+    await flushThoughtSignatureReplayForTests();
+    resetThoughtSignatureReplayForTests();
+    expect(lookupReplayThoughtSignature("call_dest_restart", scopeFor())).toBe(SIGNATURE);
+  });
 });