fix(github-copilot): strip encrypted_content from reasoning replay items#95493
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 13, 2026, 10:48 PM ET / July 14, 2026, 02:48 UTC. Summary PR surface: Source +3, Tests +1. Total +4 across 2 files. Reproducibility: yes. The source path is explicit, and an affected OpenClaw 2026.7.1 aged Telegram session reproduced the provider rejection before the boundary patch and completed repeated HTTP 200 turns afterward; the PR branch itself was not used for that installed-runtime proof. Review metrics: none identified. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Refresh and land the narrow Copilot final-payload sanitizer with its focused tests, while reviewing #95587 separately as optional defense-in-depth for failures that still surface during Responses creation or draining. Do we have a high-confidence way to reproduce the issue? Yes. The source path is explicit, and an affected OpenClaw 2026.7.1 aged Telegram session reproduced the provider rejection before the boundary patch and completed repeated HTTP 200 turns afterward; the PR branch itself was not used for that installed-runtime proof. Is this the best way to solve the issue? Yes. The final Copilot payload hook is the narrowest owner-controlled boundary for removing token-bound provider state without weakening shared Responses replay or affecting other providers; the broader drain retry is complementary recovery rather than a replacement. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 9b823c610816. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +3, Tests +1. Total +4 across 2 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
Review history (1 earlier review cycle)
|
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
Field report from an affected Observed surface:
Local mitigation proof:
Caveat: this is field evidence from a patched installed release, not a run of this PR branch itself. But it strongly supports stripping token-bound Copilot |
Updated live proof: aged
|
clawsweeper
Bot
added
proof: sufficient
Use one provider-boundary sanitizer name for both connection-bound IDs and session-bound encrypted reasoning, and assert the final stream payload drops ciphertext.\n\nCo-authored-by: openperf <[email protected]>
b517c2b to
a13e226
Compare
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: fix(github-copilot): strip encrypted_content from reasoning replay items This is item 1/1 in the current shard. Shard 0/1. This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
|
Merged via squash.
|
…ems (openclaw#95493) * fix(github-copilot): strip encrypted_content from reasoning replay items * refactor(github-copilot): name replay sanitizer accurately Use one provider-boundary sanitizer name for both connection-bound IDs and session-bound encrypted reasoning, and assert the final stream payload drops ciphertext.\n\nCo-authored-by: openperf <[email protected]> --------- Co-authored-by: Peter Steinberger <[email protected]>
Production context: - Vision/main was failing long-lived github-copilot/gpt-5.5 sessions with generic `LLM request failed` after replaying stored Responses reasoning items containing `encrypted_content`. - The raw upstream failure observed in OpenClaw logs was: `OpenAI API error (400): 400 The encrypted content ... could not be verified. Reason: Encrypted content could not be decrypted or parsed.` - Session reset temporarily worked because it removed poisoned replay state. Root cause: - Copilot encrypted reasoning content is bound to an ephemeral Copilot connection/access token, not just the stable OpenClaw session/auth/model metadata used by the core Responses replay provenance check. - Before this commit, `sanitizeCopilotReplayResponseIds` only dropped unsafe connection-bound reasoning IDs. It kept valid/idless reasoning items with `encrypted_content`, so stale token-bound ciphertext could still be sent to Copilot on later turns. Fix shape: - At the GitHub Copilot provider payload boundary, strip `encrypted_content` from every kept `type: "reasoning"` replay item. - Preserve safe reasoning IDs and summaries so replay remains summary-only instead of deleting useful non-opaque context. - Continue dropping unsafe reasoning IDs exactly as before. - Add a narrow transport safety net so Copilot's prose 400 shape is classified as recoverable by the existing one-shot encrypted-content strip/retry path. Merge/update guidance: - Upstream analogue to compare/drop against: openclaw#95493 (`fix(github-copilot): strip encrypted_content from reasoning replay items`). If that PR or equivalent lands upstream, prefer upstream's Copilot-boundary strip and drop the matching part of this commit during rebase. - The transport classifier is an extra safety net, not the primary fix. It can be dropped if upstream has broader recovery equivalent to openclaw#95587 or another matcher covering Copilot's prose error: `encrypted content` + `could not be verified` + `decrypted or parsed`. - Keep provider scope narrow: do not replace this with a global removal of all Responses reasoning unless upstream intentionally changes replay policy. Verification already run before committing: - pnpm test extensions/github-copilot/connection-bound-ids.test.ts - pnpm exec oxfmt --check --threads=1 extensions/github-copilot/connection-bound-ids.ts extensions/github-copilot/connection-bound-ids.test.ts src/agents/openai-transport-stream.ts src/agents/openai-transport-stream.test.ts - pnpm exec oxlint extensions/github-copilot/connection-bound-ids.ts extensions/github-copilot/connection-bound-ids.test.ts src/agents/openai-transport-stream.ts src/agents/openai-transport-stream.test.ts - git diff --check - pnpm build Deployment note: - Deployed to Vision/main on 2026-06-30 by building /home/bryan/openclaw and restarting `openclaw-gateway.service` (wrapper: /home/bryan/.local/bin/openclaw-main-dev, port 18789). Health returned `{ "ok": true, "status": "live" }`. (cherry picked from commit fa8562c3214252031dc075f1cd5563b33e4f1306) (cherry picked from commit 1bb7c0d5ca1bbd13902dcc72d3897c391bf94992)
3 participants
Summary
github-copilot/gpt-5.5sessions fail with400 invalid_encrypted_contenton later turns even after the merged fixes fix(agents): provenance-bound Codex reasoning replay #84367, fix(openai): preserve opaque reasoning transcript fields #90682, fix(openai): recover invalid reasoning signatures #92941, and fix(github-copilot): preserve reasoning ids with encrypted_content #71448. Persisted session transcripts containthinkingSignatureblocks withencrypted_content, and those get replayed into subsequent Copilot requests — which the provider then rejects.sanitizeCopilotReplayResponseIds(called viaonPayloadinwrapCopilotOpenAIResponsesStream) only drops reasoning items whose IDs are unsafe/connection-bound, but it passes through all reasoning items that have valid short IDs or no ID — including those carryingencrypted_content. Meanwhile, the core replay functionprepareOpenAIResponsesReasoningItemForReplaykeepsencrypted_contentwhenencryptedReasoningReplayMetadataMatchesreturns true (same OpenClaw session, same auth profile, same model). The mismatch: the context-match check uses the stable OpenClaw session ID and auth profile ID, but Copilot'sencrypted_contentis actually bound to the ephemeral per-request Copilot access token, which rotates between turns. So the core layer thinks it can replay safely, but the provider disagrees.sanitizeCopilotReplayResponseIds, after dropping items with unsafe IDs, also stripencrypted_contentfrom any reasoning items that remain (valid ID or idless). This runs at the finalonPayloadboundary — just before the request leaves for the Copilot provider — and covers both the embedded runner transport path and the shared Responses converter path.extensions/github-copilot/connection-bound-ids.ts— one newelse ifbranch stripsencrypted_contentfrom kept reasoning items; updated comment.extensions/github-copilot/connection-bound-ids.test.ts— three tests updated to reflect the corrected behavior (IDs preserved,encrypted_contentstripped).Reproduction
github-copilot/gpt-5.5that produces reasoning output (reasoning effort enabled).thinkingSignatureblocks containingencrypted_content.prepareOpenAIResponsesReasoningItemForReplaykeepsencrypted_content(context matches: same session, same auth profile);sanitizeCopilotReplayResponseIdspasses through the item unchanged; Copilot rejects with400 invalid_encrypted_content; user seesLLM request failed.sanitizeCopilotReplayResponseIdsstripsencrypted_contentfrom reasoning items at payload send time before they reach the provider; the request succeeds with summary-only reasoning replay.Real behavior proof
Behavior addressed (#95441):
sanitizeCopilotReplayResponseIdsnow stripsencrypted_contentfrom all reasoning items in the outgoing Copilot payload, so stale connection-token-bound encrypted state never reaches the provider.Real environment tested (Linux, Node 22 — Vitest against the production
sanitizeCopilotReplayResponseIdsfunction and the full github-copilot extension suite): the three updated tests directly exercise the changed code path — items with valid IDs andencrypted_contentare stripped; idless items withencrypted_contentare stripped; items with unsafe IDs are still dropped.Exact steps or command run after this patch:
pnpm test extensions/github-copilot/connection-bound-ids.test.ts;pnpm test extensions/github-copilot;node scripts/run-oxlint.mjsandpnpm format:diffon changed files.Evidence after fix (Vitest output):
Observed result after fix:
sanitizeCopilotReplayResponseIdsreturnstrueand removesencrypted_contentfrom reasoning items with valid IDs or no ID. Items with unsafe (connection-bound long) IDs are still dropped from the input array. Items with noencrypted_contentfield pass through unchanged.What was not tested: a live end-to-end
github-copilot/gpt-5.5channel session with a rotated Copilot token — the failing setup requires private Copilot credentials and an aged session transcript. The changed function is fully covered deterministically by the updated tests against the production code path.Risk / Mitigation
encrypted_contentmay reduce reasoning continuity if Copilot uses it for multi-turn reasoning context. Mitigation: the existing retry path (createResponsesStreamWithEncryptedContentRetry) already stripsencrypted_contentoninvalid_encrypted_contenterrors, proving that requests succeed without it. This fix is proactive rather than reactive.onPayloadboundary forgithub-copilot/openai-responsescalls; other providers are unaffected. Reasoning summaries (non-encrypted content) are preserved for context continuity.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Fixes #95441