Skip to content

fix: avoid replaying Responses item ids when store is disabled#84904

Merged
steipete merged 14 commits into
openclaw:mainfrom
zhanghang02:fix/responses-store-false-replay-ids
May 31, 2026
Merged

fix: avoid replaying Responses item ids when store is disabled#84904
steipete merged 14 commits into
openclaw:mainfrom
zhanghang02:fix/responses-store-false-replay-ids

Conversation

@zhanghang02

@zhanghang02 zhanghang02 commented May 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Avoid replaying persisted OpenAI Responses item ids when the outgoing request disables store.
  • Keep stateless reasoning replay usable by preserving encrypted reasoning content and normalizing missing summary arrays.
  • Preserve replay item ids only when direct OpenAI store-enabled wrappers opt in.
  • Cover OpenAI Responses, ChatGPT/Codex Responses, GitHub Copilot Responses, and shared/simple Responses conversion paths.
  • Carry current-main channel ingress queue CI fallout fixes needed after rebase.

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 stale rs_*, msg_*, or fc_* 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) with OPENAI_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 streamOpenAIResponses itself. The captured outgoing payload had store:false, omitted prior reasoning/message/function-call item ids, preserved encrypted reasoning, normalized summary: [], 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.ts
  • pnpm check:test-types
  • pnpm lint --threads=8
  • pnpm run lint:plugins:no-extension-test-core-imports
  • pnpm run lint:extensions:bundled
  • git diff --check origin/main...HEAD
  • git diff --check
  • /Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode branch --base origin/main -> clean, no accepted/actionable findings

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 5:03 PM ET / 21:03 UTC.

Summary
The branch threads a replayResponsesItemIds policy through OpenAI Responses converters and wrappers so store-disabled payloads omit prior reasoning/message/function-call item IDs, with focused regressions and channel ingress test seam fixes.

PR surface: Source +75, Tests +619. Total +694 across 31 files.

Reproducibility: yes. Source inspection and reported live evidence show current main can build store:false Responses requests while replaying prior provider item IDs; I did not run a fresh live repro during this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

Maintainer options:

  1. Land after accepting the replay contract (recommended)
    Maintainers can accept the intentional store-disabled idless replay behavior after latest-head CI is green, with the linked recovery work left as follow-up.
  2. Require recovery before landing
    Maintainers can pause this PR until the already-poisoned-session recovery path is merged if they want prevention and recovery to ship together.
  3. Request live Copilot proof first
    Maintainers can require a live GitHub Copilot Responses probe if unit-level Copilot coverage is not enough for the provider-specific part of the change.

Next step before merge

  • No automated repair is indicated; the next action is maintainer acceptance of the compatibility/session-state tradeoff plus latest-head CI verification.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes provider payload conversion and test seams, not secrets, workflows, dependencies, or install scripts.

Review details

Best 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 store:false Responses requests while replaying prior provider item IDs; I did not run a fresh live repro during this read-only review.

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 changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P1: The PR targets an active OpenAI Responses replay failure that can break follow-up agent/channel turns for real users until session reset.
  • merge-risk: 🚨 compatibility: The diff intentionally changes the provider replay contract by omitting stored item IDs on store-disabled Responses requests.
  • merge-risk: 🚨 session-state: The affected state is conversation replay/session continuation state, and linked reports show stale replay data can poison long-lived sessions.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and maintainer comments include redacted live OpenAI Responses output showing after-fix store:false requests omit replay IDs, preserve encrypted reasoning, and complete successfully.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and maintainer comments include redacted live OpenAI Responses output showing after-fix store:false requests omit replay IDs, preserve encrypted reasoning, and complete successfully.
Evidence reviewed

PR surface:

Source +75, Tests +619. Total +694 across 31 files.

View PR surface stats
Area Files Added Removed Net
Source 14 125 50 +75
Tests 17 718 99 +619
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 31 843 149 +694

What I checked:

  • Current main still sends store-disabled OpenAI Responses payloads through the default replay-id path: On current main, streamOpenAIResponses builds store: false payloads while calling convertResponsesMessages without a replay-id override, so current main does not yet implement the broader store-disabled item-id gate. (src/llm/providers/openai-responses.ts:188, e5acae44534d)
  • PR gates replay IDs before building the Responses input: The branch resolves the store policy before conversion and passes replayResponsesItemIds into convertResponsesMessages; store-disabled native paths therefore convert replay items without provider-owned IDs. (src/agents/openai-transport-stream.ts:2094, 20b9a19b5634)
  • PR deletes prior item IDs while keeping stateless reasoning material: The changed converter normalizes missing reasoning summaries to [], deletes reasoning IDs when replay IDs are disabled, omits assistant message IDs, and omits function-call item IDs while retaining call_id. (src/llm/providers/openai-responses-shared.ts:46, 20b9a19b5634)
  • Focused regression coverage locks the store-disabled behavior: The branch adds tests that build a store-disabled Responses payload and assert the reasoning, assistant message, and function-call replay IDs are omitted while encrypted reasoning and summary arrays remain. (src/agents/openai-transport-stream.test.ts:2781, 20b9a19b5634)
  • OpenAI dependency contract supports stateless encrypted reasoning replay: The OpenAI Node 6.39.1 Responses types document reasoning.encrypted_content as enabling reasoning items in stateless multi-turn conversations when store is false, and expose store, previous_response_id, reasoning summaries, and encrypted content on the relevant request/item shapes.
  • Codex sibling source was inspected for Responses item shape compatibility: The sibling Codex source models reasoning response items with required summary, optional encrypted content, and function-call IDs as optional, which is compatible with idless stateless replay for this OpenClaw payload layer. (../codex/sdk/python/src/openai_codex/generated/v2_all.py:2822, e93dc98a48d5)

Likely related people:

  • Ben Newell: Blame on current main shows the existing store-disabled Responses builder and converter scaffolding are from commit a88e4fb. (role: introduced current main Responses transport/converter surface; confidence: high; commits: a88e4fb7e05e; files: src/llm/providers/openai-responses.ts, src/agents/openai-transport-stream.ts, src/llm/providers/openai-responses-shared.ts)
  • Simon Peck: The merged adjacent replay-id policy in commit 6653193 owns the current main paired message-id logic this PR builds on. (role: recent adjacent replay-policy contributor; confidence: high; commits: 6653193fdb90; files: src/agents/openai-transport-stream.ts, src/llm/providers/openai-responses-shared.ts)
  • steipete: The provided PR timeline shows maintainer fixups, proof refreshes, and re-review requests on the current PR head, making this a practical follow-up routing candidate even though the source history predates the branch fixups. (role: recent branch fixer and reviewer; confidence: medium; commits: 7de9fee5fbfd, e6bf803aa757, b0cfe6861bce; files: src/agents/openai-transport-stream.ts, src/llm/providers/openai-responses-shared.ts, src/llm/providers/openai-chatgpt-responses.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 21, 2026
@clawsweeper

clawsweeper Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

✨ Hatched: 🥚 common Brave Crabkin

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: keeps receipts.
Image traits: location status garden; accessory review stamp; palette violet, aqua, and starlight; mood sleepy but ready; pose balancing on a branch marker; shell brushed metal shell; lighting soft underwater shimmer; background small review tokens.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Brave Crabkin in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@zhanghang02
zhanghang02 force-pushed the fix/responses-store-false-replay-ids branch from 487490e to c4c36ac Compare May 22, 2026 01:41
@zhanghang02

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 22, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 22, 2026
@latensified

Copy link
Copy Markdown
Contributor

Cross-linking the Responses item-id coordination note from #85277:
#85277 (comment)

For this PR, the proposed final layering is that the store policy gate wins first: when the final Responses payload is store:false, omit all provider-owned item ids (reasoning.id, assistant message.id, and function_call.id). When a wrapper explicitly sends store:true, id replay can remain enabled and the per-item safety rules from #85277 decide whether an assistant message.id is still replayable.

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.

@tianxiaochannel-oss88

Copy link
Copy Markdown
Contributor

This PR still looks like the closest source-level fix for a fresh 2026.5.28 macOS/Slack incident I hit today.

Live evidence from the local gateway:

  • OpenClaw CLI/Gateway: 2026.5.28 (e932160)
  • Channel: Slack channel session
  • Provider/API path: openai / openai-responses
  • Model: gpt-5.5
  • Slack health stayed OK: running, connected, health:healthy, works
  • Gateway health stayed OK: Connectivity probe: ok
  • The assistant turn failed before reply with:
HTTP 404: Item with id 'rs_[redacted]' not found. Items are not persisted when `store` is set to false. Try again with `store` set to true, or remove this item from your input.

I recovered the local install with a two-layer workaround:

  1. Reset the affected Slack channel session to clear the already-poisoned replay state.
  2. Added a local transport retry guard that treats thinking_signature_invalid and Item with id ... rs_... not found / store=false as replay-invalid, strips stale Responses replay state (reasoning items, encrypted_content, and provider-owned replay ids), and retries once.

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 store:false requests from sending provider-owned item ids in the first place, while preserving store-enabled replay where appropriate.

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.

@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 31, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 31, 2026
@openclaw-barnacle openclaw-barnacle Bot added the channel: feishu Channel integration: feishu label May 31, 2026
@steipete

Copy link
Copy Markdown
Contributor

Force-pushed one more lint-only fixup at 20b9a19.

Delta since the previous proof comment:

  • avoided the CI-only await lint in the WhatsApp runtime mocks.
  • fixed bundled Feishu runtime type exports that lint:extensions:bundled exposed.

Fresh proof after this fixup:

  • pnpm run lint:extensions:bundled
  • pnpm lint --threads=8
  • pnpm check:test-types
  • git diff --check and git diff --check origin/main...HEAD
  • autoreview branch pass against current origin/main: clean, no accepted/actionable findings.

CI restarted on 20b9a19; watching it now.

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels May 31, 2026
@steipete
steipete force-pushed the fix/responses-store-false-replay-ids branch from 20b9a19 to 5f3e7af Compare May 31, 2026 21:12
@steipete

Copy link
Copy Markdown
Contributor

Rebased on latest origin/main again and force-pushed 5f3e7af.

Delta since the last comment:

  • fixed the merged-base task boundary failure by routing async task runner lookups through task-status-access instead of importing task-registry directly.
  • fixed the remaining Feishu bundled lint await warning.

Fresh proof after this rebase:

  • 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.ts src/tasks/task-boundaries.test.ts
  • pnpm run lint:extensions:bundled
  • pnpm lint --threads=8
  • pnpm check:test-types
  • git diff --check and git diff --check origin/main...HEAD
  • autoreview branch pass against current origin/main: clean, no accepted/actionable findings.

CI restarted on 5f3e7af; watching it now.

@steipete

Copy link
Copy Markdown
Contributor

Land-ready on 5f3e7af.

Local proof:

  • 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.ts src/tasks/task-boundaries.test.ts
  • pnpm run lint:extensions:bundled
  • pnpm lint --threads=8
  • pnpm check:test-types
  • pnpm run lint:plugins:no-extension-test-core-imports
  • git diff --check and git diff --check origin/main...HEAD
  • live OpenAI Responses gpt-5.5 through streamOpenAIResponses: store:false, reasoning/message/function-call ids omitted, encrypted reasoning preserved, summary array present, request completed with stopReason=stop.
  • autoreview branch pass against current origin/main: clean, no accepted/actionable findings.

CI proof:

  • GitHub PR checks are clean for 5f3e7af. The remaining canceled Real behavior proof entry is from a superseded force-push, not the current head.

Known gap:

  • No live GitHub Copilot Responses request from this machine; Copilot-specific behavior is covered by unit regression tests for idless reasoning replay and oversized-id filtering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling channel: feishu Channel integration: feishu channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call channel: whatsapp-web Channel integration: whatsapp-web extensions: github-copilot merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants