Skip to content

Revert "fix(codex): restore prompt cache continuity for Codex requests"#2400

Merged
luispater merged 1 commit intodevfrom
revert-2374-codex-cache-clean
Mar 29, 2026
Merged

Revert "fix(codex): restore prompt cache continuity for Codex requests"#2400
luispater merged 1 commit intodevfrom
revert-2374-codex-cache-clean

Conversation

@luispater
Copy link
Copy Markdown
Collaborator

Reverts #2374

@luispater luispater merged commit 6570692 into dev Mar 29, 2026
2 checks passed
@luispater luispater deleted the revert-2374-codex-cache-clean branch March 29, 2026 14:19
Copy link
Copy Markdown
Contributor

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

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 refactors the Codex executor to simplify prompt cache handling by removing the codexContinuity logic and the associated diagnostics logging. Key changes include the explicit removal of the prompt_cache_retention field from request bodies and the standardization of session-related header keys to PascalCase. However, the refactoring of the WebSocket executor introduced an inconsistency: the applyCodexPromptCacheHeaders function no longer handles the openai source format because the context parameter was removed, preventing access to the API key required for cache key generation.

}

func applyCodexPromptCacheHeaders(ctx context.Context, auth *cliproxyauth.Auth, from sdktranslator.Format, req cliproxyexecutor.Request, opts cliproxyexecutor.Options, rawJSON []byte) ([]byte, http.Header, codexContinuity) {
func applyCodexPromptCacheHeaders(from sdktranslator.Format, req cliproxyexecutor.Request, rawJSON []byte) ([]byte, http.Header) {
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

This function's signature was changed to remove the context.Context parameter, which prevents cache key generation for openai source format since it can no longer access the API key from the context. This creates an inconsistency with cacheHelper in codex_executor.go, where the logic for openai was preserved.

To ensure consistent caching behavior between HTTP and WebSocket executors, please restore the context.Context to the function signature, update the call sites, and re-introduce the openai handling logic.

  1. Update this function's signature (line 764):
    func applyCodexPromptCacheHeaders(ctx context.Context, from sdktranslator.Format, req cliproxyexecutor.Request, rawJSON []byte) ([]byte, http.Header) {
  2. Update the call on line 193:
    body, wsHeaders := applyCodexPromptCacheHeaders(ctx, from, req, body)
  3. Update the call on line 388:
    body, wsHeaders := applyCodexPromptCacheHeaders(ctx, from, req, body)
  4. Add the openai logic back after the openai-response case (around line 789):
    	} else if from == "openai" {
    		if apiKey := strings.TrimSpace(apiKeyFromContext(ctx)); apiKey != "" {
    			cache.ID = uuid.NewSHA1(uuid.NameSpaceOID, []byte("cli-proxy-api:codex:prompt-cache:"+apiKey)).String()
    		}
    	}

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13aa5b3375

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

body, _ = sjson.SetBytes(body, "model", baseModel)
body, _ = sjson.SetBytes(body, "stream", true)
body, _ = sjson.DeleteBytes(body, "previous_response_id")
body, _ = sjson.DeleteBytes(body, "prompt_cache_retention")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve prompt_cache_retention for Responses requests

This now drops prompt_cache_retention before forwarding the request body, so clients sending OpenAI Responses payloads with "prompt_cache_retention":"persistent" silently lose persistent prompt-cache behavior. That changes runtime behavior (cache lifetime/cost profile) for valid inputs and breaks workflows that rely on retention being honored; this field should be preserved when present instead of being unconditionally deleted.

Useful? React with 👍 / 👎.

Comment on lines 785 to 789
} else if from == "openai-response" {
if promptCacheKey := gjson.GetBytes(req.Payload, "prompt_cache_key"); promptCacheKey.Exists() {
cache.ID = promptCacheKey.String()
continuity = codexContinuity{Key: cache.ID, Source: "prompt_cache_key"}
}
} else if from == "openai" {
continuity = resolveCodexContinuity(ctx, auth, req, opts)
cache.ID = continuity.Key
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Restore openai prompt-cache continuity for websocket flow

applyCodexPromptCacheHeaders no longer handles from == "openai", so websocket requests translated from OpenAI chat payloads no longer receive a stable prompt_cache_key/session identifier from request context. Because applyCodexWebsocketHeaders then fills Session_id with a fresh UUID, each turn is treated as a new session on websocket transport, which regresses cross-turn cache continuity compared with the HTTP executor path.

Useful? React with 👍 / 👎.

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.

1 participant