Skip to content

Fix replay-invalid session recovery#85362

Closed
dredozubov wants to merge 1 commit into
openclaw:mainfrom
dredozubov:codex/replay-invalid-session-recovery
Closed

Fix replay-invalid session recovery#85362
dredozubov wants to merge 1 commit into
openclaw:mainfrom
dredozubov:codex/replay-invalid-session-recovery

Conversation

@dredozubov

@dredozubov dredozubov commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Classify OpenAI/Codex invalid_encrypted_content failures as replay-invalid session state.
  • Return the specific stale-session recovery message instead of the generic runner failure copy.
  • Reset/rotate the poisoned reply session with the existing resetReplyRunSession path before returning the recovery guidance.

This is the cleaner follow-up to #85294. It deliberately leaves current main's native Codex Responses replay contract intact: safe reasoning replay remains enabled, while upstream's existing transport sanitizer/retry continues to strip unsafe encrypted content where appropriate.

Verification

  • git diff --check
  • node scripts/run-vitest.mjs src/agents/pi-embedded-helpers.isbillingerrormessage.test.ts src/auto-reply/reply/agent-runner-execution.test.ts
  • node scripts/run-vitest.mjs run --config test/vitest/vitest.e2e.config.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts -t "rotates poisoned replay-invalid sessions"
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo

Real behavior proof

Behavior addressed: A native OpenAI Codex encrypted replay failure should not leave the reply agent bound to a poisoned session that keeps failing with generic "Something went wrong" responses.

Real environment tested: Local OpenClaw checkout at PR head, using the real resetReplyRunSession helper against an on-disk OpenClaw session store rooted at /tmp/openclaw-live-proof with OPENCLAW_STATE_DIR=/tmp/openclaw-live-proof/state.

Exact steps or command run after this patch: Ran a local node --import tsx --input-type=module transcript that created a poisoned sessions.json, invoked resetReplyRunSession with the observed invalid_encrypted_content failure class, then inspected the persisted session store with plain node.

Evidence after fix: Terminal output from the local OpenClaw filesystem run:

Replay-invalid session state redacted. Restarting session main -> 77af034a-01f9-44b1-8c0c-020c8abb9913.

{
  "resetApplied": true,
  "beforeSessionId": "poisoned-session",
  "afterSessionId": "77af034a-01f9-44b1-8c0c-020c8abb9913",
  "oldBindingStillActive": false,
  "staleRuntimeFieldsCleared": {
    "systemSent": false,
    "contextTokens": null,
    "modelProvider": null,
    "model": null
  },
  "sessionFile": "/tmp/openclaw-live-proof/state/agents/main/sessions/77af034a-01f9-44b1-8c0c-020c8abb9913.jsonl",
  "userFacingRecovery": "Session history got out of sync. Please try again, or use /new to start a fresh session."
}

Observed result after fix: The OpenClaw session store moved from poisoned-session to a fresh session id, the old transcript path is no longer the active binding, stale runtime fields were cleared, and the recovery copy is the stale-session guidance instead of the generic failure copy.

What was not tested: No production Telegram/OpenClaw VM turn or live OpenAI Codex backend request was run, because that would require production bot and model credentials. The PR also does not change the native Codex Responses transport replay sanitizer that is already present on main.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed May 29, 2026, 12:59 AM ET / 04:59 UTC.

Summary
The PR classifies OpenAI/Codex invalid encrypted-content failures as replay-invalid, routes those pre-reply failures through resetReplyRunSession, returns stale-session guidance, and adds classifier, runner, and e2e coverage.

PR surface: Source +33, Tests +111. Total +144 across 6 files.

Reproducibility: yes. at source level: current main does not classify invalid_encrypted_content as replay-invalid and has no replay-invalid pre-reply reset branch. I did not run a live upstream OpenAI/Codex failure, but the supplied proof exercises the reset path after the failure class is injected.

Review metrics: 1 noteworthy metric.

  • Session-state fallback: 1 added replay-invalid reset trigger. The new trigger can rotate stored reply sessions and transcript bindings, so maintainers should notice it before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Coordinate or rebase after the linked Responses replay PRs so the final replay-invalid helper and retry/reset layering are single-source.
  • Record maintainer acceptance that replay-invalid pre-reply failures may rotate persisted reply sessions and clean old transcript state.

Risk before merge

  • [P2] Merging intentionally rotates persisted reply session bindings and cleans old transcript state when replay-invalid failures reach pre-reply handling, so maintainers should explicitly accept that session-state fallback boundary.
  • [P2] The adjacent Responses replay PRs are still open and the discussion recommends sequencing them before this fallback, so a final rebase may be needed to avoid duplicate classifier/helper contracts.

Maintainer options:

  1. Coordinate the replay-stack merge order (recommended)
    Review or land the linked Responses replay PRs first, then rebase this fallback so invalid encrypted-content classification shares the final helper and remains last-resort recovery.
  2. Accept the fallback boundary now
    Maintainers can merge this as-is if they are comfortable rotating poisoned reply sessions even before the transport-level replay fixes land.
  3. Pause if transport-only recovery wins
    Hold or close this PR if maintainers decide poisoned reply sessions should not be reset after the transport retry stack is finalized.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer review of the session-state fallback boundary and related replay PR sequencing.

Security
Cleared: No concrete security or supply-chain concern found; the diff touches runtime classification/session reset logic and tests, with no dependency, workflow, secret, permission, or package-resolution changes.

Review details

Best possible solution:

Keep the branch as a last-resort reply-session recovery path, but land it only after the replay-stack coordination confirms transport retry remains first and the final replay-invalid classifier is shared rather than forked.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main does not classify invalid_encrypted_content as replay-invalid and has no replay-invalid pre-reply reset branch. I did not run a live upstream OpenAI/Codex failure, but the supplied proof exercises the reset path after the failure class is injected.

Is this the best way to solve the issue?

Yes, with sequencing caveat: using the existing resetReplyRunSession path is the narrowest maintainable fallback for poisoned reply sessions. The safer pre-merge state is to coordinate with the related transport replay PRs so this stays a last-resort fallback and shares the final replay-invalid helper.

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 3c5f5efc8c41.

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal output from a local OpenClaw on-disk session store using the real reset helper; it does not include a live upstream OpenAI/Codex request, but it shows the changed reset behavior.

Label justifications:

  • P2: This is a normal-priority agent/session recovery bug fix with focused blast radius but real user-facing failure recovery impact.
  • merge-risk: 🚨 session-state: The PR changes how replay-invalid failures mutate persisted reply session state by rotating the active session and cleaning old transcript state.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body supplies after-fix terminal output from a local OpenClaw on-disk session store using the real reset helper; it does not include a live upstream OpenAI/Codex request, but it shows the changed reset behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body supplies after-fix terminal output from a local OpenClaw on-disk session store using the real reset helper; it does not include a live upstream OpenAI/Codex request, but it shows the changed reset behavior.
Evidence reviewed

PR surface:

Source +33, Tests +111. Total +144 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 34 1 +33
Tests 3 111 0 +111
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 145 1 +144

What I checked:

Likely related people:

  • steipete: GitHub commit history for the central agent and OpenAI transport files shows the large agent-runtime internalization and several recent provider/session-runtime touches in the same area. (role: major refactor owner and recent area contributor; confidence: high; commits: bb46b79d3c14, 7c4601ec73fc, 3e351b718e28; files: src/auto-reply/reply/agent-runner-execution.ts, src/auto-reply/reply/agent-runner.ts, src/agents/openai-transport-stream.ts)
  • pfrederiksen: Recently changed agent-runner-execution.ts for stale Codex auth recovery guidance, adjacent to this PR's pre-reply failure handling. (role: recent adjacent contributor; confidence: medium; commits: 77fe36bb98ea; files: src/auto-reply/reply/agent-runner-execution.ts)
  • fuller-stack-dev: Authored the recent compaction/session preservation work that changed related reply-session reset and recovery behavior. (role: session recovery contributor; confidence: medium; commits: cd1cae5be9ba; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/agent-runner-execution.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.

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. 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. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels May 22, 2026
@clawsweeper

clawsweeper Bot commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg: ✨ hatched 🥚 common Tiny Test Hopper. Rarity: 🥚 common. Trait: sleeps inside passing CI.

Details

Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Tiny Test Hopper in ClawSweeper.
Hatchability:

  • 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.

About:

  • Eggs appear after real-behavior proof passes. They are collectible flavor only.
  • Review momentum changes the shell state: follow-up work warms it, re-review makes it wobble, and a clean final review lets it hatch.
  • 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.

@clawsweeper clawsweeper Bot mentioned this pull request May 22, 2026
Merged
25 tasks
@latensified

latensified commented May 22, 2026

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 to keep replay-invalid session reset as the last-resort fallback when poisoned state reaches reply-agent failure handling, not as a replacement for #84267's recoverable OpenAI Responses transport retry. If #84267 drops replayable reasoning on retry, #85277 should also omit the signed assistant message.id whose paired reasoning was dropped.

Suggested merge order from the coordination pass: #84904 -> #85277 -> #84267 -> #85362. When rebasing with #84267, the clean combined shape should keep one exported isOpenAIResponsesReasoningReplayInvalidErrorMessage helper and include invalid_encrypted_content in that helper so transport retry and session-reset classification share the same OpenAI Responses encrypted-content error contract.

This was referenced May 22, 2026
Merged
Closed
@dredozubov dredozubov mentioned this pull request May 23, 2026
Closed
@clawsweeper clawsweeper Bot mentioned this pull request May 26, 2026
Closed
@dredozubov
dredozubov force-pushed the codex/replay-invalid-session-recovery branch from 4762fde to f7c9a33 Compare May 28, 2026 06:40
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 28, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 28, 2026
@BingqingLyu

This comment was marked as spam.

Sign in to view
@dredozubov
dredozubov force-pushed the codex/replay-invalid-session-recovery branch from f7c9a33 to f2e8d98 Compare May 29, 2026 04:52
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 29, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 29, 2026
@toruvieI toruvieI mentioned this pull request Jun 4, 2026
Closed
@steipete steipete mentioned this pull request Jun 14, 2026
Merged
@steipete

steipete commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Superseded by #92941, landed as 2a6eeceb40, together with the transcript-field preservation fix in #90682.

Current main now handles both known encrypted replay rejection codes (invalid_encrypted_content and thinking_signature_invalid) with one bounded pre-stream retry at the OpenAI Responses transport boundary. Resetting and deleting the whole reply session after this provider rejection would discard otherwise valid conversation state and applies recovery outside the owner boundary, so it is no longer the best fix.

If a concrete replay failure still escapes the transport retry on current main, please open a new issue with the exact provider code and pre/post-stream timing. Thanks @dredozubov for the careful session-reset proof and recovery analysis.

@steipete steipete closed this Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dredozubov @latensified @BingqingLyu @steipete