diff --git a/open-sse/executors/codex.ts b/open-sse/executors/codex.ts index c645dbcb40..77074b0557 100644 --- a/open-sse/executors/codex.ts +++ b/open-sse/executors/codex.ts @@ -879,7 +879,7 @@ export class CodexExecutor extends BaseExecutor { // session_id header — enables prompt cache affinity on the Codex backend. // The official Codex client sets this to conversation_id (a stable UUID per session). // Ref: openai/codex codex-api/src/requests/headers.rs build_conversation_headers() - const cacheSessionId = this.getPromptCacheSessionId(credentials); + const cacheSessionId = this.getPromptCacheSessionId(credentials, null); if (cacheSessionId) { headers["session_id"] = cacheSessionId; } @@ -889,12 +889,19 @@ export class CodexExecutor extends BaseExecutor { /** * Derive a stable session ID for prompt cache affinity. - * Uses workspaceId (chatgpt account ID) as the cache partition key. - * This mirrors the official Codex client's use of conversation_id for - * prompt_cache_key and session_id header. + * Priority: per-conversation session_id/conversation_id from request body → workspaceId. + * The official Codex client uses conversation_id (a unique UUID per session), NOT + * the account-wide workspaceId. Using workspaceId caps cache hit-rate at ~49% + * because all conversations share the same cache partition. (#1643) * Ref: openai/codex core/src/client.rs line 853 */ - private getPromptCacheSessionId(credentials): string | null { + private getPromptCacheSessionId(credentials, body: Record | null): string | null { + // Prefer per-session identifiers from the client request body + const sessionId = body?.session_id ?? body?.conversation_id; + if (typeof sessionId === "string" && sessionId.length > 0) { + return sessionId; + } + // Fall back to workspaceId (account-wide) — better than nothing return credentials?.providerSpecificData?.workspaceId || null; } @@ -1065,8 +1072,9 @@ export class CodexExecutor extends BaseExecutor { // The official Codex client sets this to conversation_id (a stable UUID per session). // Ref: openai/codex core/src/client.rs line 853: // let prompt_cache_key = Some(self.client.state.conversation_id.to_string()); + // IMPORTANT: Capture session/conversation IDs BEFORE deletion below (#1643). if (!body.prompt_cache_key) { - const cacheSessionId = this.getPromptCacheSessionId(credentials); + const cacheSessionId = this.getPromptCacheSessionId(credentials, body); if (cacheSessionId) { body.prompt_cache_key = cacheSessionId; } diff --git a/tests/unit/codex-cache-key.test.ts b/tests/unit/codex-cache-key.test.ts new file mode 100644 index 0000000000..a774b27395 --- /dev/null +++ b/tests/unit/codex-cache-key.test.ts @@ -0,0 +1,89 @@ +/** + * Tests for Codex prompt_cache_key per-session behavior (#1643). + * + * Validates that getPromptCacheSessionId prefers per-conversation + * session_id/conversation_id from request body over account-wide workspaceId. + */ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +// We test the logic inline since the method is private — replicate the priority logic +function getPromptCacheSessionId( + credentials: { providerSpecificData?: { workspaceId?: string } } | null, + body: Record | null +): string | null { + const sessionId = body?.session_id ?? body?.conversation_id; + if (typeof sessionId === "string" && sessionId.length > 0) { + return sessionId; + } + return credentials?.providerSpecificData?.workspaceId || null; +} + +describe("Codex prompt_cache_key (#1643)", () => { + it("should prefer body.session_id over workspaceId", () => { + const creds = { providerSpecificData: { workspaceId: "ws-account-001" } }; + const body = { session_id: "sess-conv-unique-001" }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "sess-conv-unique-001"); + }); + + it("should prefer body.conversation_id over workspaceId", () => { + const creds = { providerSpecificData: { workspaceId: "ws-account-001" } }; + const body = { conversation_id: "conv-unique-002" }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "conv-unique-002"); + }); + + it("should prefer session_id over conversation_id", () => { + const creds = { providerSpecificData: { workspaceId: "ws-account-001" } }; + const body = { session_id: "sess-001", conversation_id: "conv-002" }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "sess-001"); + }); + + it("should fall back to workspaceId when body has no session identifiers", () => { + const creds = { providerSpecificData: { workspaceId: "ws-account-001" } }; + const body = { model: "gpt-5.5" }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "ws-account-001"); + }); + + it("should fall back to workspaceId when body is null", () => { + const creds = { providerSpecificData: { workspaceId: "ws-account-001" } }; + const result = getPromptCacheSessionId(creds, null); + assert.equal(result, "ws-account-001"); + }); + + it("should return null when neither body nor credentials have IDs", () => { + const result = getPromptCacheSessionId({}, {}); + assert.equal(result, null); + }); + + it("should return null for null credentials and empty body", () => { + const result = getPromptCacheSessionId(null, null); + assert.equal(result, null); + }); + + it("should ignore empty string session_id", () => { + const creds = { providerSpecificData: { workspaceId: "ws-001" } }; + const body = { session_id: "" }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "ws-001"); + }); + + it("should ignore non-string session_id", () => { + const creds = { providerSpecificData: { workspaceId: "ws-001" } }; + const body = { session_id: 12345 }; + const result = getPromptCacheSessionId(creds, body); + assert.equal(result, "ws-001"); + }); + + it("each conversation should get a unique cache key", () => { + const creds = { providerSpecificData: { workspaceId: "ws-shared" } }; + const conv1 = { session_id: "conv-aaaa" }; + const conv2 = { session_id: "conv-bbbb" }; + const key1 = getPromptCacheSessionId(creds, conv1); + const key2 = getPromptCacheSessionId(creds, conv2); + assert.notEqual(key1, key2, "Different conversations should have different cache keys"); + }); +});