fix: avoid replaying Responses item ids when store is disabled#84904
Conversation
|
Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 5:03 PM ET / 21:03 UTC. Summary PR surface: Source +75, Tests +619. Total +694 across 31 files. Reproducibility: yes. Source inspection and reported live evidence show current main can build Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the payload-layer prevention after maintainer acceptance of the replay-contract change and latest-head checks, while keeping already-poisoned session recovery in the linked follow-ups. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection and reported live evidence show current main can build Is this the best way to solve the issue? Yes. For the payload layer, deciding replay-ID policy before message conversion is the narrowest durable fix; retry/reset recovery for already-poisoned sessions is a separate layer tracked by related items. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against e5acae44534d. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +75, Tests +619. Total +694 across 31 files. View PR surface stats
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
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Brave Crabkin Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
487490e to
c4c36ac
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Cross-linking the Responses item-id coordination note from #85277: For this PR, the proposed final layering is that the store policy gate wins first: when the final Responses payload is Suggested merge order from the coordination pass: #84904 -> #85277 -> #84267 -> #85362. That keeps this PR as the broad persistence-contract gate, then lets the signed-message pairing rule and retry/reset behavior layer on top. |
|
This PR still looks like the closest source-level fix for a fresh Live evidence from the local gateway:
I recovered the local install with a two-layer workaround:
That local installed-dist workaround is not a proposed upstream shape by itself; it is just operational proof that store-disabled replay ids are the failing boundary. The source-level fix in this PR is the better durable direction because it prevents I would still expect a separate session-recovery layer to be useful for already-poisoned long-lived channel sessions, but this PR appears to cover the transport/payload layer that prevents new poisoned turns. |
|
Force-pushed one more lint-only fixup at 20b9a19. Delta since the previous proof comment:
Fresh proof after this fixup:
CI restarted on 20b9a19; watching it now. |
20b9a19 to
5f3e7af
Compare
|
Rebased on latest Delta since the last comment:
Fresh proof after this rebase:
CI restarted on 5f3e7af; watching it now. |
|
Land-ready on 5f3e7af. Local proof:
CI proof:
Known gap:
|
Summary
store.summaryarrays.Why
When OpenClaw sends Responses requests with
store: false, provider-owned item ids from a previous turn are not persisted. Replaying those ids in the next turn can make the provider reject the request with an item-not-found error because stored items are unavailable. Store-disabled paths should replay the useful content and encrypted reasoning state without replaying stalers_*,msg_*, orfc_*item ids.Real behavior proof
Behavior addressed: Follow-up Responses turns no longer replay server-owned reasoning/message/function-call item ids when the outgoing payload has
store: false; store-enabled direct OpenAI wrappers can still opt into item-id replay.Real environment tested: Local OpenClaw branch on Node 26 using live OpenAI Responses API (
gpt-5.5) withOPENAI_API_KEY; local unit/type/lint checks on the rebased branch.Exact steps or command run after this patch: Built a live stored setup response with reasoning + encrypted content, then sent a follow-up through
streamOpenAIResponsesitself. The captured outgoing payload hadstore:false, omitted prior reasoning/message/function-call item ids, preserved encrypted reasoning, normalizedsummary: [], and the live OpenAI request completed.Evidence after fix: Redacted live proof from the direct builder request:
{"directPayload":{"store":false,"reasoningIdOmitted":true,"encryptedContentPreserved":true,"reasoningSummaryArray":true,"messageIdOmitted":true,"functionCallIdOmitted":true,"assistantPhase":"final_answer"},"liveResult":{"stopReason":"stop","contentTypes":["text"]}}Additional live shared-converter proof completed with the same id omission and encrypted reasoning preservation, returning
status:"completed"from OpenAI Responses.Observed result after fix: The live follow-up request was accepted and completed. No raw provider ids or credentials were printed.
What was not tested: A live GitHub Copilot Responses request was not run from this machine; Copilot-specific behavior is covered by wrapper/unit regression tests for idless reasoning replay and oversized-id filtering.
Testing
node scripts/run-vitest.mjs extensions/github-copilot/connection-bound-ids.test.ts extensions/github-copilot/stream.test.ts src/llm/providers/openai-chatgpt-responses.test.ts src/llm/providers/openai-compatible-auth.test.ts src/llm/providers/openai-responses-shared.test.ts src/agents/openai-transport-stream.test.ts src/agents/embedded-agent-runner-extraparams.test.ts src/agents/openai-responses.reasoning-replay.test.tspnpm check:test-typespnpm lint --threads=8pnpm run lint:plugins:no-extension-test-core-importspnpm run lint:extensions:bundledgit diff --check origin/main...HEADgit diff --check/Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/main-> clean, no accepted/actionable findings