Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a58c15725d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| */ | ||
| const cacheOptions = llmConfig as Partial<t.OAIClientOptions> & { response_format?: unknown }; | ||
| if (cacheOptions.promptCacheKeyEnabled === true) { | ||
| cacheOptions.promptCacheKey = buildPromptCacheKey({ |
There was a problem hiding this comment.
Preserve an administrator-supplied cache key
When addParams supplies promptCacheKey, getOpenAILLMConfig retains that value but also enables promptCacheKeyEnabled; this assignment then silently replaces the configured key during every agent run. Only synthesize a key when no explicit promptCacheKey is already present, otherwise the advertised fix for administrator-provided keys does not survive the real createRun path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7. Confirmed exactly as described: addParams.promptCacheKey set both the client field and the marker, and createRun then overwrote the pinned value on every run.
Root cause was that the marker carried no record of the administrator having already settled the key. getOpenAILLMConfig now withholds it when a key is pinned, and the createRun assignment additionally guards on promptCacheKey == null so the invariant holds locally for every caller of that seam rather than depending on one producer.
Your finding also exposed a bad test: llm.spec.ts asserted only on getOpenAILLMConfig output, which is why it passed while the composed path was broken. It now asserts the marker is absent, which is the assertion that would have caught this.
Verified on bc230e1d351a6f3781bcd876c1a6a06565a5720a by the stable-prefix-shares-one-cache-key-across-conversations and changed-agent-instructions-retire-the-cache-key scenarios, which drive the key through the running app.
| * prefix reach one cache entry instead of each writing their own. | ||
| */ | ||
| const cacheOptions = llmConfig as Partial<t.OAIClientOptions> & { response_format?: unknown }; | ||
| if (cacheOptions.promptCacheKeyEnabled === true) { |
There was a problem hiding this comment.
Honor dropParams when synthesizing the cache key
When an endpoint lists promptCacheKey in dropParams, deleteConfigParam removes the key but leaves the separate promptCacheKeyEnabled marker intact. This condition later observes that marker and recreates the supposedly dropped key, so dropParams does not provide the final removal asserted by the new test; dropping the key must also clear or suppress the marker.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7, same root cause as #discussion_r4014063933 and correct as reported: deleteConfigParam removes promptCacheKey but never the separately named marker, so createRun recreated a key the operator had dropped.
The marker now reads dropParams directly, because the drop cascade runs below that point and cannot see it. Grouped with the pinned-key case rather than patched separately, since both are the same defect — the marker not carrying the administrator's decision about the key.
You are also right that the existing test asserted something weaker than it claimed; it now asserts the marker is absent as well as the key.
Verified on bc230e1d351a6f3781bcd876c1a6a06565a5720a.
| if ( | ||
| firstPartyEndpoint && | ||
| promptCacheExplicit === true && | ||
| supportsExplicitPromptCache(llmConfig.model) |
There was a problem hiding this comment.
Allow explicit caching for aliased Azure models
For Azure groups whose visible model name is an alias such as production-chat for a GPT-5.6 deployment, this capability check runs against that alias before Azure replaces it with the deployment name. Consequently promptCacheExplicit: true is silently ignored for a supported model unless the administrator happens to include gpt-5.6 in the visible alias; the Azure path needs capability metadata or must trust the explicit administrator opt-in.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7. Confirmed: the capability check ran before Azure replaces the visible model with the deployment, so an alias such as production-chat hid a supported deployment and promptCacheExplicit: true was silently ignored.
It now also reads azure.azureOpenAIApiDeploymentName, and the capability pattern accepts the dash form (gpt-5-6) because Azure deployment names cannot contain a dot — which is why testing the alias alone could never have worked there.
Of your two suggested directions I kept the gate rather than trusting the opt-in outright: one Azure endpoint can host mixed deployments, and OpenAI rejects unknown body parameters rather than ignoring them, so a blanket opt-in would 400 the older ones. A fully opaque alias mapping to an opaque deployment name is still undecidable from names; that limitation is recorded in the closeout rather than papered over.
Covered by two new llm.spec.ts cases: an alias whose deployment is supported, and one where neither is.
| cacheOptions.promptCacheKey = buildPromptCacheKey({ | ||
| model: cacheOptions.model, | ||
| instructions: systemContent, | ||
| toolDefinitions, |
There was a problem hiding this comment.
Include direct tool schemas in the cache identity
For ordinary built-in, action, and provider tools, ToolService.js places the model-bound instances in agent.tools while toolDefinitions contains only the classification/event-driven definitions. Hashing only this array therefore gives agents that differ solely in those direct tool schemas the same cache bucket even though their wire prefixes differ, reducing or defeating the intended reuse; derive the identity from the schemas of both model-bound tool surfaces.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7, though the mechanism you named does not hold on this head, and the distinction matters for anyone reading this later.
ToolService.js does not place built-in, action or MCP instances in agent.tools here: every production loader is definitions-only (Endpoints/agents/initialize.js:638-647 and :1274-1283 pass true; controllers/agents/openai.js:119 and responses.js:171 default it to true), so ToolService.js:1603-1605 returns loadToolDefinitionsWrapper, whose result has no tools key at all. Those schemas were already in toolDefinitions and already hashed. The branch you described is real code but unreachable from any caller.
Your conclusion was right anyway, through two sources you did not name: provider-native tools ({ type: 'web_search' }, carried as AgentInputs.tools) and graph tools (ask_user_question, stripped from toolDefinitions, plus the run-file tools appended after the old hash site). AgentContext.getEventDrivenToolsForBinding binds all three together.
The key now hashes [...toolDefinitions, ...graphTools, ...tools], built below the run-file append so nothing lands after it, with runtime instances projected to name and description because a Zod schema is neither stable JSON nor safe for canonicalize to walk. PROMPT_CACHE_KEY_VERSION bumped to 2.
Verified on bc230e1d351a6f3781bcd876c1a6a06565a5720a.
| clientOverrides.firstPartyEndpoint ??= false; | ||
| clientOverrides.modelKwargs ??= {}; | ||
| clientOverrides.reasoning ??= undefined; | ||
| clientOverrides.promptCacheKey ??= undefined; |
There was a problem hiding this comment.
Clear cache keys for same-endpoint summaries
For ordinary self-summarization, initializeAgent sets agent.endpoint to the provider, so shapeSummarizationConfig takes its isSameEndpointAsAgent branch and never executes this cleanup. The SDK then reuses the agent client options containing the synthesized stable-prefix promptCacheKey for a summarization request that sends a different prefix, mixing unrelated prompts in one cache bucket; the same-endpoint branch also needs an explicit key-clearing override.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7. Confirmed: ordinary self-summarization takes the isSameEndpointAsAgent branch, which returns clientOverrides: undefined, so the cleanup in resolveSummarizationProvider never ran and the summarizer inherited a key naming a prefix it does not send.
promptCacheKey is now cleared on the resolved summarization parameters when the summarizer shares the agent's provider and the agent carries a synthesized key. Scoped to the synthesized key on purpose: one an administrator pinned through addParams is an endpoint-wide choice and stays.
Worth recording that the obvious fix was wrong. Adding the override unconditionally at the branch turned parameters from undefined into an object for every self-summary and broke three existing expectations; the condition keeps the plain path untouched, and run-summarization.test.ts now pins the single override that a first-party agent does carry.
Verified on bc230e1d351a6f3781bcd876c1a6a06565a5720a.
| # # claude-3.5-haiku: | ||
| # # deploymentName: claude-3-5-haiku@20241022 # Override for this model | ||
|
|
||
| # openAI: |
There was a problem hiding this comment.
Consolidate the duplicate OpenAI example block
The example already defines an endpoints.openAI block for custom headers at line 751, and this second block encourages operators to uncomment both when configuring headers and prompt caching. That produces duplicate YAML mapping keys, which js-yaml rejects rather than merging, so a configuration assembled from the two documented examples cannot load; add these fields to the existing OpenAI example instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e457f09ce2f0f06ffc5b8e87b25596d3c2d6dc7. Correct, including the consequence: endpoints.openAI already existed at line 751 for the headers example, and js-yaml rejects the duplicate mapping key rather than merging, so a config assembled from both documented blocks could not load.
The prompt-cache fields are merged into that existing block; there is now exactly one endpoints.openAI example under endpoints:.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e457f09ce
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| model: cacheOptions.model, | ||
| instructions: systemContent, | ||
| boundTools: [...toolDefinitions, ...(graphTools ?? []), ...(tools ?? [])], | ||
| responseSchema: cacheOptions.response_format, |
There was a problem hiding this comment.
Hash Responses API output formats
When a Responses-based endpoint supplies structured output through the supported text.format option (for example via addParams.text), changing that schema leaves this digest unchanged because only the Chat Completions-style response_format is hashed. This assigns different wire output schemas the same cache identity, contrary to the key contract; include the active Responses text.format value as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fdb6e27f83cfd28ba43b1d08a9aad8312a6d785a. Correct, and it broke the key's own stated contract: a Responses request carries its structured output schema as text.format (llmConfig.text, reachable through addParams since text is a known param), so changing it left the digest unchanged.
Both are now hashed under their own name rather than collapsed behind a precedence rule, so neither API's schema can borrow the other's identity. modelKwargs.text is deliberately excluded: applyResponsesVerbosity puts only verbosity there, which is not part of the cached prefix and would split the cache whenever a user toggles it.
Covered by a new promptCache.spec.ts row and verified on bc230e1d351a6f3781bcd876c1a6a06565a5720a.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc230e1d35
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cacheOptions.promptCacheKey = buildPromptCacheKey({ | ||
| model: cacheOptions.model, | ||
| instructions: systemContent, | ||
| boundTools: [...toolDefinitions, ...(graphTools ?? []), ...(tools ?? [])], |
There was a problem hiding this comment.
Build the cache key after finalizing tools
When background tasks or isolated subagents are enabled, this hashes the pre-transformation tool list: registerBackgroundTaskTool adds definitions later at line 2677, while buildIsolatedAgentInputs and the self-spawn path strip background/intent definitions after inheriting the computed key. Requests with different final wire tool prefixes can therefore share a cache bucket, and changes to those late-added or removed tools do not retire the key; compute the digest from each finalized AgentInputs after all tool transformations.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a3e3fa771da0040c4d9e39d00b5909ebd70247ab. Confirmed on both paths you name: registerBackgroundTaskTool reassigns agentInput.toolDefinitions at run.ts:2688, after buildAgentInput had already hashed, and buildIsolatedAgentInputs strips inherited background and intent definitions after calling toInput.
Rather than move the call site again, the computation is now extracted as finalizePromptCacheKey(input) and stamped onto each finished AgentInputs exactly once — after the background-task registration for a top-level agent, and at the end of buildIsolatedAgentInputs, which is the single funnel every isolated child input passes through (the self-spawn, eager-child, graph-member and prebuilt paths all call it).
This is the same cause as #discussion_r4015112637 and, in the previous round, #discussion_r4014063965 and #discussion_r4014909063: the key was being decided from a mid-flight snapshot instead of the finished request. Treating it structurally rather than patching a fourth site is the point of the change.
Verified on a3e3fa771da0040c4d9e39d00b5909ebd70247ab.
| if ( | ||
| firstPartyEndpoint && | ||
| promptCacheExplicit === true && | ||
| (supportsExplicitPromptCache(llmConfig.model) || | ||
| supportsExplicitPromptCache(azure ? azure.azureOpenAIApiDeploymentName : undefined)) | ||
| ) { | ||
| llmConfig.promptCacheExplicit = true; |
There was a problem hiding this comment.
Gate addParams explicit caching by model
When addParams.promptCacheExplicit is true for an unsupported first-party model such as gpt-4o, the earlier known-parameter loop has already copied that value into llmConfig; this condition merely declines to assign a new value and never removes the existing one. The SDK consequently emits the explicit cache parameters that this capability check is intended to withhold, causing the provider to reject the request, so all configuration sources must pass through the model gate.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in a3e3fa771da0040c4d9e39d00b5909ebd70247ab. This was the more serious of the two, because it produces a provider rejection rather than a cache miss, and you have the mechanism exactly right: promptCacheExplicit is in knownOpenAIParams, so the addParams and defaultParams loops assign it onto llmConfig before the capability check ever runs, and that check only declined to set a value it never removed.
On a first-party surface an unsupported model now has the field removed rather than merely not added. A gateway still keeps whatever it is configured with, which is this change's stated non-goal and the reason the removal is scoped rather than global.
The existing test passed because it drove the function parameter, not the parameter sources. A new case covers both addParams and defaultParams against gpt-4o.
Verified on a3e3fa771da0040c4d9e39d00b5909ebd70247ab.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3e3fa771d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if (options.promptCacheKeyEnabled === true && options.promptCacheKey == null) { | ||
| const { graphTools } = input as AgentInputs & { graphTools?: GenericTool[] }; | ||
| options.promptCacheKey = buildPromptCacheKey({ |
There was a problem hiding this comment.
Give sanitized self-spawns their own cache key
When self-spawn is enabled with background or intent tools, the sanitized agentInputs copy at lines 1953-1969 retains the parent's clientOptions object and is never passed to finalizePromptCacheKey; this assignment later mutates that shared object using the parent's unsanitized tools, so the isolated child still inherits a key for a different wire tool prefix. Fresh evidence after the prior finalization finding is that moving synthesis later exposed this shared-reference path rather than giving the sanitized copy an independently finalized options object.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 189c62f7245c42d10cf25baa230f4e491f091724. Confirmed: the sanitized self-spawn entry was a shallow spread of the parent input, so clientOptions was literally the parent's object, and finalizing the parent stamped this child with a key naming the tools the child strips. The child now takes its own clientOptions copy and is finalized against the tools it actually sends.
Your framing is right that the previous round's change is what exposed this — moving synthesis later turned a shared reference from harmless into a defect. Same cause as #discussion_r4015112635 and #discussion_r4015112637.
Verified on 189c62f7245c42d10cf25baa230f4e491f091724.
| options.promptCacheKey = buildPromptCacheKey({ | ||
| model: options.model, | ||
| instructions: input.instructions, | ||
| boundTools: [...(input.toolDefinitions ?? []), ...(graphTools ?? []), ...(input.tools ?? [])], |
There was a problem hiding this comment.
Include generated subagent tools in the cache identity
When an agent enables, disables, or changes its subagent configurations, the SDK injects or reshapes the model-facing subagent tool (packages/data-provider/src/schemas.ts:405 explicitly notes that disabling the feature prevents that injection), but this finalized projection hashes only toolDefinitions, graphTools, and tools. Because agentInput.subagentConfigs is assigned before finalization yet omitted here, materially different subagent tool prefixes retain the same cache key instead of retiring it.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 189c62f7245c42d10cf25baa230f4e491f091724. Verified in the SDK rather than inferred: Graph.ts:5164-5170 turns a non-empty subagentConfigs into executable configs and :2841-2844 adds the delegation tool to the model-facing set, so it is generated from those entries and never appears in toolDefinitions, graphTools or tools. Enabling, disabling or retargeting subagents therefore changed the wire prefix while the key stood still.
The finalized projection now includes each entry's name and description. Only that identity is hashed, not the child inputs: a child's prefix is keyed on the child's own request, and pulling child state into the parent's key would over-partition it.
Verified on 189c62f7245c42d10cf25baa230f4e491f091724.
| const explicitCacheSupported = | ||
| supportsExplicitPromptCache(llmConfig.model) || | ||
| supportsExplicitPromptCache(azure ? azure.azureOpenAIApiDeploymentName : undefined); |
There was a problem hiding this comment.
Gate explicit caching on the finalized Azure deployment
When AZURE_USE_MODEL_AS_DEPLOYMENT_NAME=true, the actual Azure deployment is replaced later with the sanitized visible model, but this capability check still accepts the originally configured deployment name. For example, visible model gpt-4o plus configured deployment gpt-5-6 enables promptCacheExplicit, after which lines 1107-1109 route the request to the unsupported gpt-4o deployment; the explicit parameters can then make the provider reject the request. Compute the gate from the same finalized deployment selection used for the wire request.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 189c62f7245c42d10cf25baa230f4e491f091724. Confirmed, including the exact route: AZURE_USE_MODEL_AS_DEPLOYMENT_NAME=true replaces the configured deployment with the sanitized visible model at llm.ts:1112-1113, after the gate had already accepted the configured name — so gpt-4o behind a gpt-5-6 deployment enabled explicit caching and then addressed the unsupported deployment.
Rather than widen the check again, the gate is now a closure invoked once the wire identity is final, at both the non-Azure and the Azure return path. That is the same structural correction as the other two findings in this round: decide from the finished request, not a mid-flight snapshot.
One consequence worth recording: running after the drop cascade, the gate also had to re-honor dropParams: ['promptCacheExplicit'], otherwise it reinstated what the cascade had removed. An existing test caught that before this was pushed.
Verified on 189c62f7245c42d10cf25baa230f4e491f091724.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 189c62f724
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22d33ddb61
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
b519c83 to
4c78aab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c78aab79a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9d3479a to
cb6cd88
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb6cd8833d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (typeof llmConfig.user === 'string') { | ||
| llmConfig.promptCacheScopeId = llmConfig.user; |
There was a problem hiding this comment.
Derive user cache scope from the authenticated user
When a first-party endpoint configures a constant addParams.user, the earlier parameter loop overwrites the authenticated user ID before these lines capture it, so every user receives the same supposedly user-scoped cache key and the documented accounting/probing boundary collapses. Fresh evidence after the resolved dropped-user thread is that promptCacheScopeId is still copied from the mutable wire parameter after addParams processing; derive it from the authenticated createRun user instead.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e932a86e18024c97e08f9347f0663dd23c7c7b69 (the derivation landed in 9d7d6243b1). Confirmed exactly as reported: promptCacheScopeId was copied from llmConfig.user after the addParams loop, so a constant addParams.user collapsed every user onto one entry — the second way that field could be rewritten after the previous round closed the dropParams one.
Reading the field back was the mistake, so it is no longer read at all. getOpenAILLMConfig stops producing a partition identity, and createRun stamps it in buildAgentInput from the run's authenticated user:
const cacheOptions = llmConfig as Partial<t.OAIClientOptions>;
if (cacheOptions.promptCacheKeyEnabled === true && typeof user?.id === 'string') {
cacheOptions.promptCacheScopeId = user.id;
}That closure is the toInput every subagent input is built through, so a delegated child carries the same partition rather than inheriting a blank one.
Verified on the published head: run-promptCache.test.ts drives the real createRun and pins addParams: { user: 'tenant-fixed' } (asserting the pinned value really reached clientOptions.user), dropParams: ['user'], and a delegated child — all three keep two users on different keys. Through the app, @scenario:two-users-of-one-agent-get-separate-cache-keys gives two authenticated users byte-identical agents and asserts the keys differ.
| */ | ||
| agents: memberConfigs.map((member) => | ||
| sealSubagentInputs( | ||
| prebuiltGraphInputs?.get(member.id) ?? buildIsolatedAgentInputs(member, toInput), |
There was a problem hiding this comment.
Clone shared graph inputs before sealing each team
When the same saved agent is a member of two graph subagent definitions with different outgoing edges, this map returns the same AgentInputs object for both. The first sealSubagentInputs call computes a key and deletes its enable marker, so the second graph cannot recompute the key and retains an identity for the first graph's transfer tools. Fresh evidence after the resolved saved-team thread is the direct reuse of the cached mutable input here; each graph occurrence needs an independently sealable copy.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e932a86e18024c97e08f9347f0663dd23c7c7b69, and the mechanism turned out to be two things rather than one — the second of which you found again a round later as #discussion_r4041186982.
You are right that the shared object was sealed once: sealSubagentInputs writes the key and deletes the marker that allows it, so the second occurrence could not recompute. ownSealableInputs now gives each occurrence its own shell (the built tool arrays and registry stay shared, so the build is still done once):
agents: memberConfigs.map((member) =>
sealSubagentInputs(
ownSealableInputs(prebuiltGraphInputs?.get(member.id) ?? buildIsolatedAgentInputs(member, toInput)),
[],
outgoingHandoffEdges(definition.edges, member.id),
),
),The part worth recording is that the divergence you predicted should not have existed in the first place. A saved team's edges are edgeType: 'direct' by schema (GraphSubagentEdge), and a direct edge creates automatic routing rather than an lc_transfer_to_* tool — packages/api/src/agents/tools.ts skips exactly those when it builds the model-facing tool allowlist. Hashing them was the real defect: it split a member across teams that differ only in routing, which is the reuse this feature exists for. outgoingHandoffEdges now filters them, so both occurrences of a member describe one prefix and share one entry.
Verified on the published head: run-promptCache.test.ts builds one member into two teams with different direct edges and asserts both sealed keys exist and agree, plus a pair of cases pinning that a handoff edge retires the key while a direct one does not.
| typeof options.modelKwargs?.model === 'string' ? options.modelKwargs.model : options.model; | ||
| options.promptCacheKey = buildPromptCacheKey({ | ||
| model: wireModel, | ||
| instructions: input.instructions, |
There was a problem hiding this comment.
Include stable additional instructions in the cache identity
When a saved agent's additional_instructions or an isolated child's always-apply skill body changes while its base instructions and tools remain unchanged, the model receives a different system prefix but this digest remains identical because it hashes only input.instructions. buildIsolatedAgentInputs explicitly appends always-apply skill bodies to additional_instructions, so a skill edit is not covered by the deployment-level key version; preserve the exclusion of volatile memory/file context while separately hashing the stable additional-instruction sources.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e932a86e18024c97e08f9347f0663dd23c7c7b69 (derivation in 9d7d6243b1), for the always-apply half, with the volatile half deliberately still out.
You separated the two correctly and that separation is the whole fix. input.additional_instructions is the dynamic system tail: prepareRuntimeAgent appends shared run context, memory and file context to it every turn, so hashing it would give every conversation its own entry and leave nothing to reuse. It stays excluded, with that reason recorded in the disposition map.
The stable source folded into it is captured where it is still separable — the one place that knows which half is configuration:
childInputs.additional_instructions = [childInputs.additional_instructions, skillInstructions]…
const childOptions = childInputs.clientOptions as Partial<t.OAIClientOptions> | undefined;
if (childOptions != null) {
childOptions.promptCacheStableInstructions = skillInstructions;
}The marker is hashed and then deleted with the others, so it never reaches the wire. A primary agent needs no equivalent: its always-apply skills are injected as conversation messages by injectSkillPrimes, not into its instruction prefix.
Verified through the app on the published head: @scenario:always-apply-skill-edit-retires-the-child-key creates an always-apply skill, attaches it to a delegated child, reads the child's key, PATCHes the skill body, and asserts the child's key retired.
| options.promptCacheKey = buildPromptCacheKey({ | ||
| model: wireModel, | ||
| instructions: input.instructions, |
There was a problem hiding this comment.
Separate Chat and Responses cache identities
When the same model and agent switch between Chat Completions and the Responses API—for example by changing addParams.useResponsesApi—this call produces the same key whenever no structured-output format distinguishes the requests. Those APIs serialize the instruction and tool prefix through different wire shapes, so retaining one identity can route or account two different prefixes together; include the finalized options.useResponsesApi mode in the hashed payload.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9d7d6243b1485f39e6056d42c4fdf3f9d4e47dba, and not by adding a sixth field to the list.
You have it right that the two APIs serialize one instruction and tool prefix into different wire shapes, so one identity must not cover both. That was the fifth model-facing surface this key had to be told about after the fact, which is the actual defect: nothing failed when a field was missing. The derivation is now exhaustive by construction — buildPromptCacheKey takes the finished AgentInputs and walks it against a total Record<keyof AgentInputs, …>, where every field is either hashed or excluded with the reason it cannot change the prefix, and an SDK upgrade that adds a field fails the typecheck in promptCache.ts instead of quietly leaving the digest.
clientOptions is projected the same way — everything except a declared non-prefix set (credentials, transport, sampling, the cache levers) participates — so useResponsesApi, response_format and text.format all enter the identity, with model resolved to the modelKwargs override Azure Astra actually addresses. A field present at runtime but absent from the map is hashed rather than dropped, so a newer SDK than these types partitions the cache (a miss) instead of pointing two prefixes at one entry.
The exclusions are tested too, because each one is a claim: per-request transport stays out (resolveConfigHeaders resolves ${conversationId} into configuration.defaultHeaders), and reasoning effort and verbosity stay out because moving those sliders does not change what the model reads.
Verified on the published head by @scenario:responses-api-switch-retires-the-cache-key, which flips the agent's useResponsesApi through PATCH /api/agents/:id and asserts a different key across the switch.
Every OpenAI request already carries `user: <userId>`, which OpenAI routes prompt-cache lookups on. Two users running the same agent therefore sit in separate cache partitions and each pay to write the same stable prefix. Requests to first-party OpenAI and Azure OpenAI now carry a deterministic `prompt_cache_key` derived from the stable instruction prefix, the tool schemas and the output schema, so requests sharing a prefix share one cache entry. Changing an agent's instructions or tools yields a new key, so nothing needs invalidating. Also adds `promptCacheRetention` and `promptCacheExplicit` as endpoint levers and routes all three through `knownOpenAIParams`: as unknown params they landed in `modelKwargs`, where Chat Completions overwrites `prompt_cache_key` with `undefined`.
Review of a58c157 found the synthesized key describing a request other than the one being made, in four ways: - An administrator key pinned through `addParams`, or removed through `dropParams`, was overwritten by `createRun` on every run: the enabled marker carried no record of that decision. It is now withheld whenever the key is already settled. - The key hashed only `toolDefinitions`, missing provider-native tools (`{type:'web_search'}`) and graph tools (`ask_user_question`, run files), which the SDK binds alongside. It now hashes the whole bound surface, built after the run-file tools exist, projecting runtime instances to name and description because a Zod schema is neither stable JSON nor safe to walk. - Self-summarization inherited the agent's key while sending a different prefix, filing unrelated prompts under one identity. - Azure aliases hid a supported deployment from the explicit-cache capability check, which now also reads the deployment name and its dash form. The `endpoints.openAI` example is merged into the existing block: two commented blocks under one mapping produce a duplicate key that js-yaml rejects. Adds the mock-harness scenarios that exercise the key through the app.
The specs only held on desktop-light. `sendAssertion` navigated to a new chat and `openAgentBuilder` navigated again, so on the second send of a test the option click landed on a detaching overlay and timed out. One navigation remains, and the composer is awaited before a send. Mobile is skipped for the reason the neighbouring Agent Builder scenarios skip it: the builder panel is desktop-only.
A Responses request carries its structured output schema as , not , so changing that schema left the digest unchanged and two different wire output schemas shared one cache identity. Both fields are now hashed under their own name, so neither API can borrow the other's.
Saving an agent refetches the agent list. A refetch landing mid-click re-renders the listbox and detaches the option under the pointer, so Playwright's own click retry retried against a dead node and the instruction-change scenario timed out on desktop-dark. The whole open-and-choose is retried instead.
Two variants of one cause: both the key and the capability gate read a request that was not final yet. The key was built inside , but tools move afterwards — adds definitions to the parent, and isolated children strip inherited background and intent definitions. Different wire prefixes could therefore share an identity. now stamps each finished , once, after those transformations. is a known parameter, so assigned it straight onto and bypassed the model gate entirely, sending explicit cache parameters to models that reject the request. On a surface whose contract we own the gate now removes it rather than declining to add it.
Three more variants of the same cause, each narrower than the last. The explicit-cache gate read the configured Azure deployment, but AZURE_USE_MODEL_AS_DEPLOYMENT_NAME replaces it afterwards, so a visible model behind a deployment enabled explicit caching and then routed to the unsupported deployment. The gate now runs once the wire identity is final, at both return paths, and re-honors an explicit drop since it outlives the drop cascade. The sanitized self-spawn copy shared the parent's clientOptions object by shallow spread, so finalizing the parent stamped the child with a key naming tools the child strips. It owns its options and is finalized against what it sends. Delegation is a model-facing tool the SDK generates from subagentConfigs rather than from the tool arrays, so enabling, disabling or retargeting subagents changed the wire prefix without retiring the key.
Azure Astra keeps its visible identity in and sends the deployment through the override, so hashing alone gave two Astra deployments one identity. The override is now preferred when present. The self-summarization cleanup ran after user parameters merged, so it also erased a the summarization config set for itself. Only the inherited key is cleared now.
Per-user cache partitioning is a property of the request LibreChat already sends, not an accident of it: `user: <userId>` gives every user their own cache accounting, and on GPT-5.6 and later OpenAI states that separate keys are what prevent one user detecting, from a cache hit, that another already sent a prompt they can guess. The synthesized `prompt_cache_key` now carries that partition by default, so the feature no longer trades the boundary away to win a cache write. The new `promptCacheScope` lever opts into the trade: `shared` drops the user from the key so one cached prefix serves everyone running the same agent. Routing is unaffected for GPT-5.6 and later, where OpenAI routes cache lookups itself; models before it still get a stable per-user key to route on, which is what the `user` field was already doing.
computed a child cache key before the caller recursed for that child every own spawn target, so a child that delegates advertised a delegation tool its key did not describe. Two siblings differing only in their own descendants then shared an identity while sending different wire prefixes. The key is now sealed where the input actually becomes final. Both the eager and the lazily resolved path attach descendants through , graph members seal on construction because a graph delegates through its own edges, and returns a draft and says so.
A multi-agent graph turns each outgoing handoff edge into a model-facing tool, but edges arrive on after the identity was sealed, so retargeting one changed the wire tool prefix without retiring the key. Each agent now hashes its own outgoing edges, projected onto what the model sees of them — target, description and input parameter — while the server-side stays out. Tool projection was an allowlist of name, description and parameters, which dropped the classification fields puts on every MCP definition. withholds a tool from the binding and can keep it off the direct surface, so either one flipping changes the prefix. A definition is plain JSON and is now hashed whole; the name-and-description projection is reserved for runtime instances, whose Zod schemas cannot be walked.
The delegation tool the SDK generates accepts `subagent_type`, whose values come from each config`s `type` — the agent id for eager and lazy children. Projecting a config onto name and description alone meant swapping a child for a different agent that shares a display name changed the accepted enum while the cache identity stayed put. `type` now enters the identity. `agentInputs`, `maxTurns` and `allowNested` stay out: they govern execution and never reach the model.
The per-user partition read `options.user` at finalization, but `dropParams: ['user']` removes that field and the gpt-4o search models drop it unconditionally — so those requests hashed an empty scope and every user silently collapsed onto one shared entry, which is precisely what the default exists to prevent. The identity is now captured when the policy is resolved, before anything can remove the field, and travels as its own marker. Saved-team members were sealed with an empty handoff list even though the graph definition supplies the edges that become their transfer tools, so editing a team edge left every member key unchanged. Members now seal with the edges that leave them, through the same helper the top level uses.
Four rounds found the same defect in four places: the cache identity was assembled from a list of fields written at a call site, so every model-facing surface nobody thought of — the delegation tool, the handoff edges, the subagent type enum, the Responses output format, the API mode — shared an identity with a prefix it no longer described. Patching a fifth field would have left the sixth. `buildPromptCacheKey` now takes the finished `AgentInputs` and walks it against a total `Record<keyof AgentInputs, …>` disposition map: a field is hashed, or it is excluded with the reason it cannot change the prefix. An upgrade that adds a model-facing field fails the typecheck in `promptCache.ts` instead of quietly leaving the digest, and a field present at runtime but absent from the map is hashed rather than dropped, so a newer SDK partitions the cache (a miss) instead of colliding (a wrong identity). `clientOptions` is projected the same way, which is how `useResponsesApi` enters the identity: Chat Completions and the Responses API serialize one prefix into two wire shapes. The partition identity now comes from the authenticated user of the run, stamped in `buildAgentInput`. Reading it back off the request could never be right: `addParams` pins `user` to a constant, `dropParams` removes it, and the `gpt-4o*search` models drop it on their own — each of which merges every user of an agent onto one entry, which is what the per-user default exists to prevent. An isolated child's always-apply skill bodies are recorded where they are still distinguishable from the memory and file context they are joined to, so editing such a skill retires the child's key while the volatile tail stays out. `ownSealableInputs` gives each occurrence of a shared input its own shell before sealing, so a saved-team member listed by two teams with different edges no longer keeps the first team's identity, and the self-spawn child no longer depends on a hand-written clone at one call site.
The four prompt-cache levers read `endpoints.all` before the endpoint's own block, so a value written under `openAI` or `azureOpenAI` was ignored whenever a global default existed — the opposite of what the comment above it and `librechat.example.yaml` both promise, and the opposite of what an operator configuring one endpoint expects. `endpoints.all` is now the fallback.
cb6cd88 to
1dba5be
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dba5be8a9
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return (edges ?? []) | ||
| .filter((edge) => (Array.isArray(edge.from) ? edge.from : [edge.from]).includes(agentId)) | ||
| .map(handoffEdgeIdentity); |
There was a problem hiding this comment.
Exclude direct edges from the prompt-cache identity
For saved subagent teams, whose schema restricts edges to edgeType: 'direct', this helper still adds every outgoing edge to the digest. Fresh evidence beyond the earlier edge comments is that the production tool allowlist explicitly skips direct edges in packages/api/src/agents/tools.ts:364-367, matching the GraphEdge contract that only handoff edges create lc_transfer_to_* tools. Consequently, the same member used in teams that differ only in automatic routing gets separate cache keys despite an identical model-facing prefix, reducing the reuse this feature is intended to provide; filter out direct edges before hashing.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
.filter((edge) => (Array.isArray(edge.from) ? edge.from : [edge.from]).includes(agentId))
.map(handoffEdgeIdentity);
}
handoffEdgeIdentityinstructionsrun-promptCache.test.tshandoffdirect` edge does not, a spelled-out default name agrees with an omitted one, and one member in two teams with different direct edges keeps one identity.
EOF
)
There was a problem hiding this comment.
Fixed in e932a86e18024c97e08f9347f0663dd23c7c7b69. Correct, with the evidence you cite: only a handoff edge becomes an lc_transfer_to_* tool, packages/api/src/agents/tools.ts skips edgeType: 'direct' when it builds the model-facing allowlist, and a saved team's edges are direct by schema (GraphSubagentEdge).
function outgoingHandoffEdges(edges: readonly GraphEdge[] | undefined, agentId: string): unknown[] {
return (edges ?? [])
.filter((edge) => edge.edgeType !== 'direct')
.filter((edge) => (Array.isArray(edge.from) ? edge.from : [edge.from]).includes(agentId))
.map(handoffEdgeIdentity);
}This is the same cause as #discussion_r4030918204, in the opposite direction: that finding read the split as a missing identity, and it was really a surplus one. Hashing automatic routing partitioned exactly the case the feature exists to serve — one agent reused across teams — so filtering it dissolves the team-occurrence divergence rather than papering over it. The per-occurrence copy introduced for that finding stays, because sealing still has to be occurrence-local.
Folded in from the same reading: handoffEdgeIdentity now resolves the handoff parameter name rather than passing it through, since the SDK falls back to instructions and an edge that spelled the default out was partitioning away from one that left it unset while advertising the same tool.
Verified on the published head: run-promptCache.test.ts pins that a handoff edge retires the key, a direct edge does not, a spelled-out default name agrees with an omitted one, and one member listed by two teams with different direct edges keeps a single identity.
(The earlier reply on this thread was posted truncated by a shell quoting error on my side — this is the complete answer.)
| provider === fallbackProvider && | ||
| agentParameters?.promptCacheKeyEnabled === true && | ||
| parameters?.promptCacheKey == null | ||
| ) { | ||
| parameters = { ...parameters, promptCacheKey: undefined }; |
There was a problem hiding this comment.
Re-gate explicit caching for same-endpoint summaries
When a GPT-5.6 agent has endpoint-level promptCacheExplicit: true but its same-endpoint summarizer selects an unsupported model such as gpt-4o, the summarization path reuses the agent's client options and this cleanup clears only promptCacheKey. The inherited promptCacheExplicit: true therefore survives onto the summary client, bypassing the model gate in getOpenAILLMConfig and causing the provider to reject the explicit cache parameters once summarization triggers; recompute or explicitly clear this flag for the summary model unless the summary parameters intentionally supply their own value.
AGENTS.md reference: AGENTS.md:L22-L27
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e932a86e18024c97e08f9347f0663dd23c7c7b69. This was the more serious of the two findings in this round, because it ends in a provider rejection rather than a cache miss, and you have the route exactly right: the capability gate in getOpenAILLMConfig ran against the agent's model, the same-endpoint branch supplies no client overrides, and the summary client therefore inherited promptCacheExplicit: true past its own gate.
The cleanup now covers the flag as well as the key, scoped the same way — an inherited value is withheld, a value the summarization config chose for itself survives:
if (
provider === fallbackProvider &&
agentParameters?.promptCacheExplicit === true &&
userParameters?.promptCacheExplicit == null &&
!supportsExplicitPromptCache(model)
) {
parameters = { ...parameters, promptCacheExplicit: undefined };
}Gated on the summary model rather than cleared unconditionally on purpose: a same-provider summarizer that runs the agent's own GPT-5.6 model sends a prefix that does accept the explicit controls, and there is no reason to withhold them there.
Verified on the published head by a new run-summarization.test.ts case: a first-party GPT-5.6 agent with endpoint-level promptCacheExplicit: true summarizing on gpt-4o ends with both promptCacheExplicit and promptCacheKey cleared on the summary parameters.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4ae7494296
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const field of PROMPT_CACHE_ADMIN_FIELDS) { | ||
| delete (llmConfig as Record<string, unknown>)[field]; | ||
| } |
There was a problem hiding this comment.
Strip cache controls from fallback client options
When an agent author supplies model_parameters.fallbacks[].clientOptions, the free-form agent schema preserves the nested object and withModelCallbacks confirms those options are used to construct fallback clients. This loop removes administrator-only fields only from the top-level llmConfig, so a fallback can still carry an author-chosen promptCacheKey, billed promptCacheRetention: '24h', or unsupported promptCacheExplicit; once the primary fails, those values bypass the policy enforced here and reach the provider. Recursively strip PROMPT_CACHE_ADMIN_FIELDS from fallback client options before they are retained.
Useful? React with 👍 / 👎.
An agent whose instructions use `{{current_date}}` has the resolved text moved
into the dynamic tail, which is what keeps today's date out of the cached
prefix — but it also took the instructions themselves out of the identity, so
editing them left the key standing. The unresolved template is recorded
instead: editing retires the key, the clock does not.
`useResponsesApi` hashes as one of the two states the wire has. `false` and
absence both select Chat Completions, so an agent that spells the default out
now reuses the entry of one that does not.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cc295f17f7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const supported = | ||
| supportsExplicitPromptCache(llmConfig.model) || supportsExplicitPromptCache(deploymentName); |
There was a problem hiding this comment.
Gate explicit caching on the modelKwargs wire model
When a first-party OpenAI agent has a supported visible model such as gpt-5.6 but supplies an unsupported wire override such as model_parameters.modelKwargs.model: 'gpt-4o', this check enables promptCacheExplicit based only on the visible model. The request and the cache identity both treat modelKwargs.model as the effective wire model, so GPT-4o receives the GPT-5.6-only explicit cache parameters and rejects the request. Include the finalized llmConfig.modelKwargs.model override in this capability gate.
Useful? React with 👍 / 👎.
| const identities: unknown[] = []; | ||
| for (const tool of value) { | ||
| const name = (tool as { name?: unknown } | null)?.name; | ||
| const configured = typeof name === 'string' ? state[name] : undefined; |
There was a problem hiding this comment.
Restrict configured-tool lookups to own properties
When a configured tool is named toString or constructor and any other tool has been discovered in the conversation, state[name] resolves an inherited Object.prototype member even though no state was recorded for that tool. The projection consequently treats the tool as discovery-mutated and deletes its defer_loading classification, allowing deferred and eager versions of the model-visible tool surface to hash to the same key. Use an own-property check or a null-prototype record before restoring configured state.
Useful? React with 👍 / 👎.
| * token placeholders into `configuration.defaultHeaders`, so hashing the | ||
| * transport would partition the cache per conversation and per turn. | ||
| */ | ||
| const nonPrefixClientOptionKeys: ReadonlySet<string> = new Set([ |
There was a problem hiding this comment.
Exclude service tier from the prompt identity
When an administrator changes service_tier between values such as default, flex, or priority, clientOptionsIdentity hashes the setting because it is absent from this exclusion set. The tier changes request scheduling and cost, not the instructions, tool schemas, or output schema that form the cached prefix, so this produces a new deterministic key and forfeits an otherwise reusable cache entry. Exclude service_tier alongside the other non-prefix routing and sampling options.
Useful? React with 👍 / 👎.
A fallback client is constructed from its own `clientOptions`, and the administrator-only strip covered the top level alone — so an agent author could carry a pinned key, billed `24h` retention, or unsupported explicit caching in `fallbacks[].clientOptions` and have it reach the provider the moment the primary client failed. The strip recurses now. The explicit-cache gate read the visible model while `modelKwargs.model` overrides it on the wire, so a supported visible name fronting an unsupported override would have had the request rejected. It gates on every name the request can address, the way the digest already keys on them. Two narrow gaps beside them: a tool named after an inherited object property resolved a prototype member in the configured-state lookup and lost its own classification from the identity, and the service tier — scheduling and cost, never the prefix — was retiring keys.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6379cf5486
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const key of Object.keys(record).sort()) { | ||
| const item = canonicalize(record[key], seen); | ||
| if (item !== undefined) { | ||
| normalized[key] = item; |
There was a problem hiding this comment.
Preserve
__proto__ in canonicalized objects
When a model-facing JSON schema contains an own property named __proto__—for example, an OpenAPI action parameter with that name—assigning it onto this ordinary object invokes the legacy prototype setter instead of creating an own property. The field then disappears from JSON.stringify(canonicalize(payload)), so schemas with and without that parameter can receive the same prompt-cache key despite exposing different tool definitions; construct normalized with a null prototype or define keys as data properties.
Useful? React with 👍 / 👎.
| configuredToolState[toolName] = existingToolNames.has(toolName) | ||
| ? { deferLoading: toolDef.defer_loading } | ||
| : { appended: true }; |
There was a problem hiding this comment.
Record
__proto__ tool discovery state as an own property
When a deferred tool named __proto__ is discovered, this assignment changes the prototype of configuredToolState rather than recording an enumerable entry. Consequently the later Object.keys check can omit the marker, and the definition mutated by overrideDeferLoadingForDiscoveredTools hashes differently before and after discovery, defeating cross-conversation key reuse. Fresh evidence after the own-property lookup fix is that the write still targets a normal {}; use a null-prototype record or another own-property-safe representation.
Useful? React with 👍 / 👎.
Assigning a key named `__proto__` onto an ordinary object invokes the legacy prototype setter instead of creating an own property, so a tool schema carrying that field — an OpenAPI action parameter, for one — disappeared from the canonicalized payload and hashed like a schema without it, and a deferred tool by that name set a prototype instead of recording its configured state. Every record the identity builds has a null prototype now.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3b01f0a42
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function stripPromptCacheControls(options: Record<string, unknown>): void { | ||
| for (const field of PROMPT_CACHE_ADMIN_FIELDS) { | ||
| delete options[field]; | ||
| } |
There was a problem hiding this comment.
Strip raw cache controls from modelKwargs
When a Responses agent author places wire-level fields such as prompt_cache_key, prompt_cache_retention, or prompt_cache_options under the free-form model_parameters.modelKwargs, this sanitizer only removes the camel-case controls from the outer options object and fallback clients. Those nested kwargs are forwarded as request parameters, allowing an author to pin a shared cache key, request billed retention, or bypass the explicit-cache capability gate despite these being endpoint-admin controls; sanitize the raw wire spellings inside every modelKwargs object as well.
AGENTS.md reference: AGENTS.md:L66-L68
Useful? React with 👍 / 👎.
| const projectedValue = | ||
| key === 'modelKwargs' ? modelKwargsIdentity(options[key]) : safeIdentity(options[key]); |
There was a problem hiding this comment.
Exclude top-level Responses verbosity from the cache identity
When a Responses agent supplies the native shape text: { verbosity: 'low', format: ... }, changing only text.verbosity changes the digest here even though verbosity affects output detail rather than the cached instruction/tool prefix. The equivalent modelKwargs.text path already removes this field in modelKwargsTextIdentity; apply the same projection to top-level text so moving the verbosity setting does not unnecessarily retire an otherwise reusable cache entry while still hashing text.format.
Useful? React with 👍 / 👎.
`modelKwargs` is forwarded to the request body verbatim, so an agent author who
wrote `prompt_cache_key`, `prompt_cache_retention` or `prompt_cache_options`
there reached the provider without passing any of the policy that governs the
camelCase fields — a shared key or billed retention chosen by whoever can edit
an agent. Both alphabets are stripped now, at the top level and inside every
fallback's kwargs.
A Responses agent can also carry the native `text: { verbosity, format }`
shape, where verbosity was retiring keys although it changes output detail
rather than the prefix. The top-level `text` gets the projection its
`modelKwargs` twin already had: the format keys, the verbosity does not.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4555ab21ed
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (firstPartyEndpoint && promptCacheKeyEnabled === false) { | ||
| delete llmConfig.promptCacheKey; |
There was a problem hiding this comment.
Remove raw pinned keys when caching is disabled
When a first-party Responses endpoint has a legacy/admin addParams.prompt_cache_key and the operator sets promptCacheKey: false, addParams has already placed the raw field in the local modelKwargs, while this branch deletes only the camel-case property. The kwargs are attached to llmConfig later and Responses forwards the raw field, so the documented kill switch still sends the pinned cache key; remove prompt_cache_key from modelKwargs here as well.
Useful? React with 👍 / 👎.
`addParams` is applied after the administrator sanitizer, so an operator's raw `prompt_cache_key` sits in the request kwargs by the time the policy runs — and Responses forwards those verbatim. `promptCacheKey: false` deleted only the camelCase field, so the documented "send no key at all" still sent one. Both spellings go now.
An operator who pins `prompt_cache_key` through `addParams` had it overwritten: only the constructor field counted as pinned, so `createRun` synthesized a key over the operator's own. Both spellings count now, and `dropParams` honors either. A handoff edge's target may be a list, and the SDK emits one transfer tool per destination — so a single target, a one-element list, and two orderings of the same targets all advertise the same tools and now hash alike.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 25f638433a
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const promptCacheKeyPinned = typeof llmConfig.promptCacheKey === 'string'; | ||
| const promptCacheKeyDropped = dropParams?.includes('promptCacheKey') === true; |
There was a problem hiding this comment.
Honor raw pinned keys before synthesizing replacements
When an endpoint pins addParams.prompt_cache_key, it is stored in modelKwargs, so this camel-case-only check still enables synthesis and finalizePromptCacheKey adds a generated promptCacheKey; on Chat Completions that constructor field is serialized after modelKwargs and overwrites the administrator's raw pinned value. The same spelling mismatch means dropParams: ['prompt_cache_key'] cannot prevent synthesis. Treat the raw modelKwargs value and raw drop spelling as settled key decisions before setting promptCacheKeyEnabled.
AGENTS.md reference: AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
| for (const field of PROMPT_CACHE_WIRE_FIELDS) { | ||
| delete modelKwargs[field]; |
There was a problem hiding this comment.
Delete only the raw key when key synthesis is disabled
When promptCacheKey: false is combined with legacy/admin addParams.prompt_cache_retention, prompt_cache_options, or prompt_cache_breakpoint, this loop removes those independent controls along with prompt_cache_key. The toggle is documented as sending no key, not as disabling retention or explicit caching, and the camel-case equivalents remain independently configurable; delete only prompt_cache_key here so existing raw endpoint configuration is preserved.
AGENTS.md reference: AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
| * Null-prototype: a key named `__proto__` assigned onto an ordinary object | ||
| * invokes the legacy prototype setter instead of becoming an own property, | ||
| * so a schema carrying that field would vanish from the digest and hash like | ||
| * one without it. |
There was a problem hiding this comment.
Version the changed context fingerprint encoding
When a persisted event-actor context contains an own __proto__ field in a tool definition or another semantic record, older nodes' ordinary-object canonicalizer omitted that field while this null-prototype implementation includes it. Because AGENT_CONTEXT_FINGERPRINT_VERSION remains 1, a rolling deployment treats the unchanged context as an incompatible digest and rejects its warm continuation instead of recognizing an encoding-version transition; bump the fingerprint version or retain the old encoding specifically for that persisted contract.
AGENTS.md reference: AGENTS.md:L31-L34
Useful? React with 👍 / 👎.
The summary gate accepted either the visible model or the wire override, so a supported alias fronting an unsupported deployment kept the inherited controls and the request would be rejected. The override decides whenever it exists — permitting and vetoing — which is the precedence the configuration path already uses.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 14e978a508
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| agentParameters?.promptCacheExplicit === true && | ||
| userParameters?.promptCacheExplicit == null && | ||
| !summarySupportsExplicitCache | ||
| ) { | ||
| parameters = { ...parameters, promptCacheExplicit: undefined }; |
There was a problem hiding this comment.
Clear raw explicit controls for unsupported summaries
When a first-party Responses agent receives explicit caching through trusted addParams.prompt_cache_options rather than the camel-case promptCacheExplicit field, a same-provider summary inherits that raw value through the agent's modelKwargs. If compaction selects an unsupported model such as GPT-4o, agentParameters.promptCacheExplicit is undefined, so this branch does not clear anything and the unsupported summary request is rejected for the unknown cache parameter. Apply the summary-model gate to inherited raw explicit-cache fields as well, while preserving values explicitly supplied by the summarization configuration.
Useful? React with 👍 / 👎.
| if (firstPartyEndpoint && promptCacheRetention != null) { | ||
| llmConfig.promptCacheRetention = promptCacheRetention; |
There was a problem hiding this comment.
Honor wire-spelled retention drops
When endpoints.all or the OpenAI/Azure endpoint enables promptCacheRetention: '24h' and a model group uses the conventional provider spelling dropParams: ['prompt_cache_retention'], this assignment adds the camel-case constructor field anyway. The later deleteConfigParam call only removes that exact snake-case key from modelKwargs, so LangChain still serializes promptCacheRetention back to prompt_cache_retention and the supposedly excluded model continues using billed 24-hour retention. Treat the wire spelling as an alias for promptCacheRetention, as the key path already does for prompt_cache_key.
Useful? React with 👍 / 👎.
`promptCacheKey: false` was removing every raw cache field from the request kwargs, so an endpoint that disabled the key also lost the retention an administrator had set through `addParams`. It removes the key alone now, and the retention and explicit-cache levers keep their own policy.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86057fcbe8
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 'maxTokens', | ||
| 'maxCompletionTokens', | ||
| 'max_tokens', |
There was a problem hiding this comment.
Exclude top-level Responses output caps from the cache key
When a stored agent supplies its documented model_parameters.max_output_tokens, getOpenAILLMConfig leaves that field at the top level of llmConfig, but this exclusion list only covers maxTokens, maxCompletionTokens, and max_tokens; only the separately generated modelKwargs.max_output_tokens path is excluded later. Fresh evidence beyond the prior nested-kwargs case is the persisted AgentModelParameters.max_output_tokens contract, which therefore changes the synthesized key whenever the output-token limit changes even though it cannot change the cached prompt prefix. Exclude the top-level snake- and camel-case output-cap spellings here as well.
Useful? React with 👍 / 👎.
| * deployment that is depends on `AZURE_USE_MODEL_AS_DEPLOYMENT_NAME` and the | ||
| * base URL, neither of which is resolved yet here. | ||
| */ | ||
| const promptCacheExplicitDropped = dropParams?.includes('promptCacheExplicit') === true; |
There was a problem hiding this comment.
Honor wire-spelled drops for explicit cache controls
When an endpoint enables promptCacheExplicit but a model group uses dropParams: ['prompt_cache_options'] (or prompt_cache_breakpoint) for a model that must not receive explicit caching, this check does not recognize the wire spelling. The later generic drop only deletes that literal key from modelKwargs, while llmConfig.promptCacheExplicit remains true and the SDK regenerates the options and breakpoint, so the model-specific veto is ineffective and may produce a rejected request. Treat both emitted wire fields as aliases that disable the paired explicit-cache flag.
Useful? React with 👍 / 👎.
…trols `dropParams: ['prompt_cache_retention']` is the spelling an operator reads in OpenAI's own documentation, and it did nothing: the constructor field was set anyway and LangChain serialized it straight back to the excluded name, with billed retention attached. Both spellings drop it now, as the key path already did. A same-provider summary also inherits the agent's request kwargs whole, so explicit cache controls an administrator set through `addParams` reached a summary model that cannot accept them without passing the camelCase gate. The inherited raw fields are cleared under the same model gate, while anything the summarization config set for itself is left alone.
3143cc8 to
e66b230
Compare
A caller whose user came from a lean query or a token payload carries only the document id, and the partition was stamped from the virtual `id` alone — so those requests fell back to an unscoped identity and merged every user of an agent onto one cache entry, which is exactly what the per-user default exists to prevent. Either form now identifies the partition, and both produce the same one.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e66b230225
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * rest of `modelKwargs` is admin-configured (`addParams`) or wire identity | ||
| * (`model` carries the Azure Astra deployment), so it is hashed. | ||
| */ | ||
| const nonPrefixModelKwargsKeys: ReadonlySet<string> = new Set([ |
There was a problem hiding this comment.
Exclude wire-spelled sampling kwargs from the identity
When endpoint configuration uses provider spellings such as addParams: { top_p, frequency_penalty, presence_penalty, logit_bias }, getOpenAILLMConfig routes them into modelKwargs because knownOpenAIParams contains only their camel-case equivalents. This projection then hashes those values even though the equivalent top-level sampling controls are deliberately excluded, so changing an output-sampling setting unnecessarily retires the deterministic cache key despite an unchanged instruction/tool prefix. Add the wire-spelled sampling fields to this exclusion set.
Useful? React with 👍 / 👎.
| 'streaming', | ||
| 'streamUsage', |
There was a problem hiding this comment.
Exclude wire-spelled streaming controls from the identity
When an agent or endpoint sets the recognized stream or stream_options fields, getOpenAILLMConfig retains them on the top-level client options, but this transport exclusion covers only the LangChain spellings streaming and streamUsage. Changing streaming delivery or whether usage is included therefore produces a different synthesized cache key even though the instruction and tool prefix is identical. Exclude the two provider-spelled streaming controls here as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb34ac1d3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * rest of `modelKwargs` is admin-configured (`addParams`) or wire identity | ||
| * (`model` carries the Azure Astra deployment), so it is hashed. | ||
| */ | ||
| const nonPrefixModelKwargsKeys: ReadonlySet<string> = new Set([ |
There was a problem hiding this comment.
Exclude raw cache controls nested in modelKwargs
When trusted endpoint configuration uses provider spellings such as addParams: { prompt_cache_retention: '24h' } (or prompt_cache_options/prompt_cache_breakpoint), getOpenAILLMConfig routes them into modelKwargs, but this exclusion set hashes them. Changing only retention or explicit-cache mode therefore generates a new synthesized key even though the equivalent camel-case controls are excluded because they do not change the prompt prefix, causing an avoidable cold cache when enabling paid 24-hour retention; exclude these nested wire controls as well.
Useful? React with 👍 / 👎.
`addParams` can place `prompt_cache_options` or `prompt_cache_breakpoint` straight into the request kwargs, which are forwarded verbatim — so a model that rejects them was sent exactly the parameters the capability gate exists to withhold, and `dropParams` written in those names did nothing. The gate removes both raw fields when it withholds the flag, and either raw name counts as the drop. That closes the class: all four controls now follow one policy in both spellings — stripped from author parameters, honored from administrator configuration, gated on every model name a request can address, and cleared from an inherited summary request.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb996c3f1d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (const field of inheritedRawFields) { | ||
| if (summaryKwargs[field] == null) { | ||
| summaryKwargs[field] = undefined; |
There was a problem hiding this comment.
Clear inherited raw explicit-cache controls
For an Azure Responses agent whose endpoint addParams supplied prompt_cache_options or prompt_cache_breakpoint, a same-provider summary that preserves the API mode receives those values through resolveAzureSummarization, which spreads sourceKwargs into the summary's modelKwargs at lines 1120-1124. This condition then sees the inherited value as non-null and preserves it instead of replacing it with undefined, so selecting an unsupported summary deployment still sends the explicit-cache fields and the summary request is rejected. Fresh evidence on this head is that the attempted cleanup examines the already-merged summary kwargs rather than whether the summarization configuration itself supplied the field.
Useful? React with 👍 / 👎.
Every field in this feature has two names — LangChain's `topP` and the provider's `top_p` — and `addParams` lets an administrator write either, so the known spelling lands on the client options and the unknown one in the request kwargs. The exclusion sets named only one spelling each, which is why the identity kept partitioning on a sampling knob, an output cap, a streaming toggle or a cache lever depending on which name an operator happened to use. Exclusions are matched on a normalized form now, so a set names a field once and both spellings follow it. The summary cleanup also compares against the kwargs the summarization configuration itself supplied rather than the merged result, because `resolveAzureSummarization` spreads the agent's kwargs in and testing the merge preserved exactly the inherited field it meant to withhold.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4963850488
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return new Set(keys.map(normalizeOptionKey)); | ||
| } | ||
|
|
||
| const nonPrefixClientOptionNames: readonly string[] = [ |
There was a problem hiding this comment.
Exclude tool-choice controls from the cache identity
When an agent changes tool_choice or parallel_tool_calls, both recognized options remain in clientOptions and are hashed because this exclusion list does not contain them. These controls affect which tool calls the model may return, but they do not change the stable instructions or bound tool schemas being cached, so toggling either setting unnecessarily generates a new deterministic key and discards reusable prompt-cache entries; add both spellings to the non-prefix exclusions.
Useful? React with 👍 / 👎.
| if (options.promptCacheKeyEnabled === true && options.promptCacheKey == null) { | ||
| options.promptCacheKey = buildPromptCacheKey(input, { handoffEdges }); | ||
| } |
There was a problem hiding this comment.
Synthesize keys for first-party fallback clients
When a first-party agent configures an OpenAI fallback, withModelCallbacks preserves that fallback's independent clientOptions, but finalization stamps a key only on the outer client. Because the sanitizer also removes any author-supplied fallback cache key, a primary-model failure switches to a first-party request with no deterministic key at all, so the advertised default caching disappears precisely on the fallback path; derive a fallback-specific key from the same finished input and the fallback's own model options before constructing the run.
Useful? React with 👍 / 👎.
| recordStableInstructionText( | ||
| agent as Agent & { configuredAdditionalInstructions?: string }, | ||
| instructionTemplate, | ||
| ); |
There was a problem hiding this comment.
Preserve non-temporal substitutions in mixed templates
When instructions contain both a temporal variable and {{current_user}}, this branch sends the fully resolved text in additional_instructions but records only the entirely unresolved template in the cache identity. Since that dynamic tail is excluded, renaming a user leaves the same key for different model-facing text, and promptCacheScope: shared gives different users the same key despite their resolved names differing; resolve stable substitutions such as current_user while normalizing only the temporal placeholders before recording the identity text.
Useful? React with 👍 / 👎.
An administrator can pin `prompt_cache_key` in its wire spelling, and a same-provider summary inherits the agent's request kwargs whole — so the summary sent a key naming a stable prefix it does not send, filing unrelated summary prompts under the agent's identity. It is cleared with its camelCase twin now, unless the summarization configuration pinned one for itself, and the explicit controls keep their own model gate.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 113d29c736
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| firstPartyEndpoint && | ||
| promptCacheKeyEnabled !== false && | ||
| !promptCacheKeyPinned && |
There was a problem hiding this comment.
Keep deterministic cache keys opt-in by default
When an operator upgrades without configuring promptCacheKey, the undefined value satisfies this !== false check and enables deterministic keys on every first-party OpenAI/Azure agent request. That changes existing routing, cache accounting, and per-user partitioning despite the repository requirement that a new lever's default preserve current behavior; require an explicit true or default the schema/config to disabled.
AGENTS.md reference: AGENTS.md:L66-L68
Useful? React with 👍 / 👎.
| if (provider === fallbackProvider && parameters?.promptCacheKey == null) { | ||
| if (agentParameters?.promptCacheKeyEnabled === true) { | ||
| parameters = { ...parameters, promptCacheKey: undefined }; |
There was a problem hiding this comment.
Clear inherited camel-case keys from summaries
When a first-party endpoint pins addParams.promptCacheKey, the agent has promptCacheKey but no promptCacheKeyEnabled marker, so this branch adds no clearing override. A same-provider summarizer then inherits the agent client's pinned key even though it sends a different summary prefix; the raw prompt_cache_key path immediately below is cleared for the same reason. Clear an inherited camel-case key whenever the summarization configuration did not provide its own.
Useful? React with 👍 / 👎.
| const inheritedExplicitFields = PROMPT_CACHE_WIRE_FIELDS.filter( | ||
| (field) => | ||
| field !== 'prompt_cache_key' && | ||
| inheritedKwargs?.[field] != null && |
There was a problem hiding this comment.
Preserve raw retention on unsupported summary models
When a same-provider summarizer selects a model outside the explicit-cache allowlist and the endpoint supplied addParams.prompt_cache_retention, this filter classifies retention with the unsupported explicit controls and overwrites it with undefined. Retention is independently supported and independently configured—the camel-case promptCacheRetention path is not cleared by this gate—so the raw spelling silently loses the administrator's requested retention only on summary requests; restrict this cleanup to prompt_cache_options and prompt_cache_breakpoint.
Useful? React with 👍 / 👎.
Instructions can mix a temporal variable with `{{current_user}}`, and the
identity recorded the raw template — so two users sent different system text
under one identity, which `promptCacheScope: shared` would have handed the same
entry. The recorded text now resolves against a fixed instant: the temporal
placeholders collapse to a constant while every other substitution resolves as
the model will see it.
`tool_choice` and `parallel_tool_calls` decide which of the bound tools the
model may call, not what the schemas are, so they leave the identity.
Summary
Every OpenAI and Azure OpenAI request LibreChat sends carries
user: <userId>(packages/api/src/endpoints/openai/initialize.ts). On models before GPT-5.6 that field is what OpenAI routes prompt-cache lookups on, so one user's second conversation may not reach the machine holding the prefix it just wrote; on GPT-5.6 and later OpenAI routes automatically, and a key instead decides how cache usage is accounted and where the boundary between users sits.Requests to first-party OpenAI and Azure OpenAI now carry a deterministic
prompt_cache_keythat names one agent's configured prompt prefix and the user it is accounted to. Nothing about the conversation enters it, so a new chat or a different user message reaches the same entry; a real change to the agent's instructions, tools, delegation targets, handoff edges, output schema or API mode produces a different digest and a new cache identity, with no invalidation step. Implicit caching stays on, so growing conversation history keeps being reused as it is today.The key is scoped per user by default, which preserves the partitioning the
userfield already produces — including the property that a user cannot learn, from a cache hit, that someone else already sent a prompt they can guess.promptCacheScope: sharedopts into the other trade: one cached prefix serves everyone running the same agent, so a deployment pays one cache write instead of one per user.Three adjacent levers ship with it:
promptCacheScope,promptCacheRetention(prompt_cache_retention) andpromptCacheExplicit(GPT-5.6prompt_cache_options+ breakpoints). All arelibrechat.yamlendpoint options, settable underopenAI,azureOpenAIorendpoints.all, and the wire-level ones are now inknownOpenAIParams. That last part is a fix in its own right: as unknown params they were routed tomodelKwargs, and@langchain/openaispreadsmodelKwargsbefore the explicitprompt_cache_keyfield on Chat Completions — so an admin setting it by hand had it overwritten withundefinedand silently never sent.Gateways are untouched. The gate is the existing
firstPartyOpenAI || firstPartyAzurecomputation, which is false for OpenRouter, the Vercel gateway, everycustomendpoint and any reverse-proxy base URL. Anthropic, Bedrock, Google and Vertex are not on this path at all.Partially addresses #14949.
promptCacheExplicitdefaults off and stays off until LibreChat-AI/agents#546 lands: today the SDK places the breakpoint on the last system message, which LibreChat builds by joining the stable prefix to the volatile tail, so the marked prefix turns over every turn. That PR anchors it to the stable prefix; enabling this by default is a follow-up after the dependency bump.How it works
Policy and value are resolved in different places, because no single place knows both:
getOpenAILLMConfigruns long before the agent's instructions and discovered tools exist, so it can decide whether a key is allowed but not what it is.createRunconsumes the markers at the one point where the input is final, then drops them so none reaches the wire.The identity is derived by exclusion, not from a field list
buildPromptCacheKeytakes the finishedAgentInputsand walks it against a total map overkeyof AgentInputs: every field is either hashed, or excluded with the reason it cannot change the prefix.That shape is the point. A hand-picked list at the call site is why the delegation tool, the handoff edges, the
subagent_typeenum, the Responses output format and the API mode each had to be added after the fact: nothing failed when a model-facing surface was missing. A total map makes the next upstream field a build failure inpromptCache.ts, and a field present at runtime but absent from the map is hashed rather than dropped — so a newer SDK than these types partitions the cache (a miss) instead of pointing two different prefixes at one entry (a wrong identity).clientOptionsis projected the same way: everything except a declared non-prefix set (credentials, transport, sampling, the cache levers themselves) participates, withmodelresolved to themodelKwargsoverride that Azure Astra actually addresses.useResponsesApitherefore enters the identity, because Chat Completions and the Responses API serialize one prefix into two wire shapes. The exclusions are deliberate and tested: per-request transport must stay out becauseresolveConfigHeadersresolves${conversationId}intoconfiguration.defaultHeaders, and reasoning effort and verbosity must stay out because a user moving those sliders does not change the prefix the model reads.What the key does not name is one turn's exact wire tool list. Tools a conversation discovered through
tool_search, and the dynamic system tail of memory and file context, are excluded with their reasons — a key that followed them would give every conversation its own entry and there would be nothing left to reuse.The partition comes from the authenticated user
promptCacheScopeIdis stamped inbuildAgentInputfromcreateRun's authenticated user. Theuserfield on the request is not an identity:addParams.userpins it to a constant,dropParams: ['user']removes it, and thegpt-4o*searchmodels drop it unconditionally — each of which would merge every user of an agent onto one cache entry, which is exactly what the per-user default exists to prevent.Each occurrence of a shared input is sealed on its own
Tools are still added and stripped after an input is first assembled, so the key is stamped where the input becomes final:
ownSealableInputsgives one occurrence its own shell before sealing. Sealing writes the key and removes the marker that allowed it, so an object reaching two occurrences would keep the first one's identity: a saved-team member listed by two teams with different edges, and the self-spawn child of its parent, are both sealed independently now.An isolated child's always-apply skill bodies are recorded on
promptCacheStableInstructionswhere they are still distinguishable from the memory and file context they are joined to, so editing such a skill retires the child's key while the volatile tail stays out.Adjacent fixes
resolveSummarizationProvideralready neutralizes the agent'suseResponsesApi,firstPartyEndpoint,modelKwargsandreasoningwhen a summarizer shares the agent's provider. The inheritedpromptCacheKeyjoins that list — it names a stable prefix a summarization request does not send — while a key the summarization config sets for itself survives.endpoints.allbefore the endpoint's own block, so a value written underopenAIorazureOpenAIwas ignored whenever a global default existed — the opposite of what the example config documents.endpoints.allis now the fallback.handoffEdgeIdentityresolves the handoff parameter name rather than passing it through: the SDK falls back toinstructions, so an edge that spelled the default out landed in a different partition than one that left it unset while advertising the same tool.Change Type
Testing
packages/api/src/endpoints/openai/promptCache.spec.tscovers the identity contract from both sides: what retires it (instructions, wire model including the Azure Astra deployment override, tool schemas and their classification fields, the delegation tool's name/description/subagent_type, handoff edges, both output-schema shapes, the API mode, the scope) and what deliberately does not (the volatile tail, tools discovered in this conversation, the tool-search corpus, sampling parameters, reasoning effort and verbosity, per-request transport headers, credentials). It also pins that an unknown input field partitions rather than collides.packages/api/src/agents/__tests__/run-promptCache.test.tsdrives the realcreateRunand reads the request the SDK receives: two authenticated users never share a key while one user reuses theirs across conversations;addParams.userpinned to a constant anddropParams: ['user']both leave the partition intact; a delegated child's partition follows the authenticated user; one saved-team member listed by two teams with different edges gets one key per team; no marker reaches the wire.packages/api/src/endpoints/openai/requests.spec.tsdrives the realinitializeModelagainst a fakefetchand asserts the actual request body on all four surfaces — OpenAI Chat Completions, OpenAI Responses, Azure Chat Completions, Azure Responses — with two turns per surface sharing oneprompt_cache_key,prompt_cache_retentionandprompt_cache_optionson the wire, and no camelCase leakage.llm.spec.tscovers the policy boundary, andinitialize.spec.tsthe config precedence.Seven scenarios run through the app in the mock harness, on desktop light and desktop dark:
stable-prefix-shares-one-cache-key-across-conversationschanged-agent-instructions-retire-the-cache-keygateway-endpoint-sends-no-cache-keyresponses-api-switch-retires-the-cache-keytwo-users-of-one-agent-get-separate-cache-keysdelegated-child-keys-its-own-stable-prefixalways-apply-skill-edit-retires-the-child-keyTest Configuration:
Lighthouse medians under the 250 ms/query serial-latency hook: LCP 3,644 ms (budget 4,500), CLS 0.017 (0.1), TBT 54 ms (500).
Checklist