fix(cache): emit tools before input in OpenAI Responses request body for prefix-cache stability#78747
Conversation
openclaw-barnacle
Bot
added
agents
|
Codex review: needs changes before merge. Reviewed July 3, 2026, 5:06 PM ET / 21:06 UTC. Summary PR surface: Source +10, Tests +47, Docs +1. Total +58 across 3 files. Reproducibility: yes. source-level reproduction is high-confidence: current main constructs the Responses payload with Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Copy recommended automerge instructionNext step before merge
Security Review findings
Review detailsBest possible solution: Refresh the PR on current main by moving the surviving projected Do we have a high-confidence way to reproduce the issue? Yes, source-level reproduction is high-confidence: current main constructs the Responses payload with Is this the best way to solve the issue? No as submitted: the fix belongs in this transport path, but the branch must be rebased onto the current projected-tool implementation instead of replacing it. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 010b61746379. Label changesLabel justifications:
Evidence reviewedPR surface: Source +10, Tests +47, Docs +1. Total +58 across 3 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
+1 from independent production traffic — same symptom, different model. Running an agent ( Sample of 370688 → 27136 Twelve such drops in a 24h window, all on the same End-to-end latency on this agent: median 21s, p90 78s. Tool-heavy turns make 9 sequential LLM round-trips; if your paired-trial result (−3.5 s/turn) holds for gpt-5.5, that compounds to ~30 s off the long tail of replies. Happy to capture before/after numbers once this lands if useful. |
|
@georgel — thanks for posting that. The shape of your If it's useful, here's the exact capture script we used to produce the 15% → 99% byte-stable-prefix numbers in the PR body. It's a Node preload (
// fetch tap: log codex/responses calls + (toggleable) reorder body keys so
// `tools` sits before `input`. JSON semantics unchanged; on-the-wire bytes
// move so the upstream prompt cache can match the long stable prefix.
//
// Wire it in via: NODE_OPTIONS=--require=/tmp/fetch-tap.cjs in your gateway
// systemd env, then `systemctl --user restart openclaw-gateway`.
// Toggle reorder via FETCH_TAP_REORDER=1 in the same env.
const fs = require('fs');
const LOG = '/tmp/fetch-tap.log';
const REORDER = process.env.FETCH_TAP_REORDER === '1';
const orig = global.fetch;
if (orig && !orig.__tapped) {
global.fetch = async function tapped(input, init) {
const url = typeof input === 'string' ? input : input?.url || '';
const isCodex = /chatgpt\.com.*backend-api\/codex\/responses/.test(url);
const t0 = Date.now();
let bodyStr = null, reordered = false;
if (isCodex && init && init.body) {
try {
bodyStr = typeof init.body === 'string'
? init.body
: Buffer.from(init.body).toString('utf8');
} catch (_) {}
}
if (REORDER && isCodex && bodyStr) {
try {
const parsed = JSON.parse(bodyStr);
if (parsed && typeof parsed === 'object' && 'tools' in parsed && 'input' in parsed) {
const order = [
'model','store','stream','include','instructions','tools',
'tool_choice','parallel_tool_calls','prompt_cache_key',
'prompt_cache_retention','text','reasoning','metadata',
'previous_response_id','context_management','input',
];
const out = {};
for (const k of order) if (k in parsed) out[k] = parsed[k];
for (const k of Object.keys(parsed)) if (!(k in out)) out[k] = parsed[k];
init = { ...init, body: JSON.stringify(out) };
bodyStr = init.body;
reordered = true;
}
} catch (_) {}
}
const resp = await orig.call(this, input, init);
if (isCodex) {
try {
fs.appendFileSync(LOG, JSON.stringify({
t: t0, url, status: resp.status,
dur_ms: Date.now() - t0,
body_len: bodyStr ? bodyStr.length : 0,
reordered,
// Comment in if you also want the body for prefix-stability
// analysis (warning: this writes customer prompts to disk):
// body: bodyStr ? bodyStr.slice(0, 1000000) : null,
}) + '\n');
} catch (_) {}
}
return resp;
};
global.fetch.__tapped = true;
}Workflow we used:
import json
rs = [json.loads(l) for l in open('/tmp/fetch-tap.log') if 'codex/responses' in l]
b0, b1 = rs[-2]['body'], rs[-1]['body']
n = min(len(b0), len(b1))
i = 0
while i < n and b0[i] == b1[i]: i += 1
j = 0
while j < n and b0[len(b0)-1-j] == b1[len(b1)-1-j]: j += 1
print(f'common prefix: {i*100//n}% common suffix: {j*100//n}% body: {len(b0)}b')Heads-up: with Your tool-heavy 9-round-trip turns are exactly the case where this compounds; 5/5 paired wins on our (single-LLM-roundtrip) SMS turns at -3.5s mean — if it scales linearly with round-trips, you're looking at ~30s off the long tail like you said. Would love to see your numbers if you do capture them. Re-review progress:
|
openclaw-barnacle
Bot
added
proof: supplied
clawsweeper
Bot
added
the
proof: sufficient
4a1fd5e to
864dc61
Compare
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
|
Rebased and resolved conflicts. cc @steipete @vincentkoc — would appreciate a review when you have a moment. |
`buildOpenAIResponsesParams` was inserting `params.tools = ...` AFTER the literal that already contained `input: messages`. JS object property insertion order determines JSON key order, which determines the byte stream the upstream prompt cache matches against. With `input` enumerated first, the (large, per-session-stable) tools schema landed past the per-turn divergence point and was excluded from the cached prefix every turn. Hoist tool conversion above the literal and place `tools` between `stream` and `instructions`; move `input` to the end of the literal so per-turn mutations sit downstream of the cache-stable prefix. Semantics unchanged: the same fields with the same values, only their on-the-wire byte order changes. Adds a regression test that asserts `Object.keys(params)` enumerates `tools` before `input` (and double-checks via `JSON.stringify`). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
864dc61 to
efc9a95
Compare
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
the
proof: sufficient
|
This pull request has been automatically marked as stale due to inactivity. |
clawsweeper
Bot
added
rating: 🌊 off-meta tidepool
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
status: ⏳ waiting on author
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
rating: 🦐 gold shrimp
2 participants
Summary
buildOpenAIResponsesParams(src/agents/openai-transport-stream.ts:979) inserts the literal withinput: messagesfirst, then assignsparams.tools = convertResponsesTools(...)later via a conditional block. JS property insertion order determines JSON key order, which determines the byte prefix the upstream prompt cache matches. Withinputenumerated first, the (large, per-session-stable)toolsschema lands past the per-turn divergence point and is excluded from the cached prefix every turn.convertResponsesToolsabove the params literal so the converted tools can be inserted in the literal betweenstreamandinstructions. Moveinputto the end of the literal so per-turn mutations sit downstream of the cache-stable prefix. Drop the now-redundantif (context.tools) params.tools = ...block. Same fields, same values; only on-the-wire byte order changes.toolsis still produced by the sameconvertResponsesToolscall with the samestrictflag;input,metadata,prompt_cache_key,prompt_cache_retention, and the conditional reasoning/include/service_tier/temperature/max_output_tokens code paths are unchanged. No transport, auth, or sanitize-policy changes. Same applies to the Azure and Codex variants since they all flow throughbuildOpenAIResponsesParams.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
openai-responses/openai-codex-responses/azure-openai-responses.Real behavior proof
openai-codex/gpt-5.4-mini, real customer session with the standard SOUL/USER/TOOLS/IDENTITY/HEARTBEAT system prompt and 6 plugins loaded (acpx, browser, device-pair, phone-control, sendblue, talk-voice).globalThis.fetchvia aNODE_OPTIONS=--require=...preload that does the equivalent JSON-key reorder on the wire. Restarted the gateway, then sent the same 5 SMS in the same order on each side of a paired trial (REORDER OFF baseline → REORDER ON), measuring per-turnendToEndMsfrom the existing sendblue plugin timing instrumentation. Phase 2 ran after phase 1, so the session already had 5 extra turns of conversation history baked in (~70KB extrainputper phase-2 request) — that biased phase 2 slower by ~1-3s/turn, which makes the result a conservative measurement.Byte-prefix stability (single biggest signal):
Paired trial — same 5 SMS in the same order, real production agent:
5/5 wins. Mean per-turn latency dropped 3.5s.
Status code check: all 8 reordered codex calls captured during phase 2 returned
status: 200. The Codex Responses backend accepts the reordered body shape without complaint, confirming JSON semantic equivalence holds — no other plumbing reorder is needed.Captured directly from the wire (request body head, post-reorder):
vs pre-reorder:
(
toolsnow sits in the byte-stable prefix; per-turninputcontent is at the end.)TransformStream-wrappedResponseinterfered with how the OpenAI SDK consumes the response stream and was rolled back.endToEndMsfrom the sendblue plugin's existing instrumentation (request received → outbound reply sent) was used as the proxy. I have not tested the Azure or non-Codex (openai-responsesagainstapi.openai.com) variants since this PR usesopenai-codex-responsesagainstchatgpt.com— butbuildOpenAIResponsesParamsis the shared builder for all three, so the same key ordering applies.Root Cause
src/agents/openai-transport-stream.ts:1008placedinputearly; the conditionalif (context.tools) params.tools = ...block at line 1027 reassigned a property that was not in the literal, so V8 insertedtoolsat the end of the property order.JSON.stringify(used by the OpenAI SDK serialization path) emits keys in insertion order, putting the (cache-stable)toolsschema downstream of the (per-turn-mutating)inputarray on the wire. Upstream prefix cache matches byte-by-byte from offset 0, so caching stops at the first byte ofinputand never seestools.buildOpenAIResponsesParamstests assert values viatoMatchObject/toEqual, both of which are order-insensitive.cache_controlmarkers,OPENCLAW_CACHE_BOUNDARY, deterministic tool sort in fix(cache): sort MCP tools deterministically to stabilize prompt cache #58037). Codex Responses goes through a different cache mechanism — byte-prefix matching with no explicit markers — and the symmetric ordering fix wasn't there yet.Regression Test Plan
src/agents/openai-transport-stream.test.ts, newit("emits \tools` before `input` so the tools schema lives in the cacheable prefix", ...)`buildOpenAIResponsesParamscall that produces bothtoolsandinput,Object.keys(params)must enumeratetoolsbeforeinput. Belt-and-suspenders:JSON.stringify(params)must serialize"tools"before"input".Object.keys(params)directly catches any future refactor that re-introduces a lateparams.tools = ...assignment without needing to mock the SDK or upstream cache.User-visible / Behavior Changes
None. Same fields, same values. Only the on-the-wire JSON key order changes — semantically equivalent to the upstream API. The user-visible effect is faster turns once the upstream cache warms up.
Diagram
Security Impact
Repro + Verification
Environment
openai-codex/gpt-5.4-miniviachatgpt.com/backend-api/codex/responsesSteps
NODE_OPTIONS=--require=/path/to/fetch-tap.cjspreload that captures the request body for any call tochatgpt.com/backend-api/codex/responses.hi,what's the weather,tell me a joke,what time is it,thanks). Capture each turn's request body.endToEndMsfrom the existing sendblue timing log.Expected
endToEndMsdrops by several seconds on warm-cache turns.status: 200for the reordered body.Actual
Matches expected. See "Real behavior proof" above for the measured numbers.
Evidence
Human Verification
azure-openai-responses) and direct OpenAI public-API (openai-responsesagainstapi.openai.com) paths. Both flow through the samebuildOpenAIResponsesParams, so the source change applies, but I did not exercise them on real traffic.Compatibility / Migration
Risks and Mitigations
200for 8/8 captured calls. JSON ordering is not part of the documented OpenAI/Codex contract.prompt_cache_key) will be a one-turn miss for every active session immediately after this lands. Mitigation: the cache rebuild happens during the very first turn after deploy and benefits every subsequent turn within the session; observed in the paired trial as turn-1-post-reorder being slightly slower than turn-2-post-reorder.Notes
pnpm tsgo:coreerrors in unrelated files (src/media/**,src/plugins/**,src/secrets/**, etc.) are visible onmain;tsgo:corereports zero errors in the touched files (src/agents/openai-transport-stream.ts,src/agents/openai-transport-stream.test.ts).node_modules/@openclaw/fs-safe/dist/is missing afterpnpm install(the package'sexportsmap points at./dist/...but the dist artifacts aren't shipped). That's not introduced by this PR. Real-environment proof above stands in for the unit test until CI runs the suite.🤖 Generated with Claude Code