fix(gateway): repair orphaned tool_use blocks on session load#8243
fix(gateway): repair orphaned tool_use blocks on session load#8243unisone wants to merge 1 commit intoopenclaw:mainfrom
Conversation
| // See: https://github.com/openclaw/openclaw/issues/7930 | ||
| const repaired = repairToolUseResultPairing(messages as AgentMessage[]); | ||
| return repaired.messages; |
There was a problem hiding this comment.
[P2] Type-only import path looks incorrect and will break builds
session-utils.fs.ts imports AgentMessage from ../agents/types.js, but there doesn’t appear to be a corresponding src/agents/types.* file in the repo. Even though it’s a type-only import, TS still needs to resolve the module; this will fail typechecking/bundling.
If the intent is to use the same AgentMessage used by repairToolUseResultPairing, consider importing it from @mariozechner/pi-agent-core instead (or from whatever existing local module already re-exports it).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/session-utils.fs.ts
Line: 40:42
Comment:
[P2] Type-only import path looks incorrect and will break builds
`session-utils.fs.ts` imports `AgentMessage` from `../agents/types.js`, but there doesn’t appear to be a corresponding `src/agents/types.*` file in the repo. Even though it’s a type-only import, TS still needs to resolve the module; this will fail typechecking/bundling.
If the intent is to use the same `AgentMessage` used by `repairToolUseResultPairing`, consider importing it from `@mariozechner/pi-agent-core` instead (or from whatever existing local module already re-exports it).
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c32d21ad2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const repaired = repairToolUseResultPairing(messages as AgentMessage[]); | ||
| return repaired.messages; |
There was a problem hiding this comment.
Avoid mutating chat.history with repair-only transcript logic
Applying repairToolUseResultPairing() on every readSessionMessages() call means chat.history now returns a modified transcript, not what was actually stored. That repair function drops toolResult messages that appear after a later assistant turn (it only matches results in the span before the next assistant), and inserts synthetic error toolResults instead. In sessions where a long-running tool result is written after another assistant message, the real tool output will be silently removed from history and replaced with an error, which is a behavior change for UI and tools that consume chat.history (e.g., sessions_history with includeTools=true). Consider keeping chat.history as a faithful read and applying repair only in provider-context sanitization.
Useful? React with 👍 / 👎.
2c32d21 to
e5103bb
Compare
|
Re: Codex P2 (history mutation concern) Valid point. The trade-off:
If UI fidelity to raw storage matters, I can add a separate |
|
Re: Greptile P2 (import path) Fixed in latest commit — now imports |
4e57958 to
9d81cf3
Compare
Cherry-picked from upstream PR openclaw#8243 When browser operations timeout, sessions can be saved with orphaned tool_use blocks (no matching tool_result). On reload, this causes API errors: 'unexpected tool_use_id in tool_result blocks'. The repairToolUseResultPairing() function already exists and handles this case during context sanitization, but wasn't called when loading sessions from disk. Fixes openclaw#7930
When browser operations timeout, sessions can be saved with orphaned tool_use blocks (no matching tool_result). On reload, this causes API errors: 'unexpected tool_use_id in tool_result blocks'. The repairToolUseResultPairing() function already exists and handles this case during context sanitization, but wasn't called when loading sessions from disk. Fixes #7930
9d81cf3 to
cfc6bb3
Compare
|
All bot feedback addressed and CI is green across all platforms. Ready for review. |
Cherry-picked from upstream OpenClaw: - PR openclaw#9416: drop errored/aborted assistant tool pairs in transcript repair - PR openclaw#12487: strip orphaned tool_result when tool_use is sanitized on retry - PR openclaw#8243: repair orphaned tool_use blocks on session load Fixes session-killing errors caused by orphaned tool_use/tool_result blocks. When these orphaned blocks reach the API, they cause permanent HTTP 400 errors that break every subsequent message in the session.
bfc1ccb to
f92900f
Compare
|
Friendly ping — this is ready for review. Let me know if any changes needed. |
Cherry-picked from upstream OpenClaw: - PR openclaw#9416: drop errored/aborted assistant tool pairs in transcript repair - PR openclaw#12487: strip orphaned tool_result when tool_use is sanitized on retry - PR openclaw#8243: repair orphaned tool_use blocks on session load Fixes session-killing errors caused by orphaned tool_use/tool_result blocks. When these orphaned blocks reach the API, they cause permanent HTTP 400 errors that break every subsequent message in the session.
Summary
When browser operations timeout, sessions can be saved with orphaned
tool_useblocks (no matchingtool_result). On reload, this causes API errors:Root Cause
repairToolUseResultPairing()already exists insession-transcript-repair.tsand handles this case during context sanitization, but wasn't called when loading sessions from disk inreadSessionMessages().Fix
Call
repairToolUseResultPairing()on messages when loading from disk, before returning them.Testing
Fixes #7930
Greptile Overview
Greptile Summary
This PR fixes session reload failures by running
repairToolUseResultPairing()on messages loaded from disk inreadSessionMessages(), ensuring any orphaned/misorderedtool_use/tool_resultpairs are repaired before the transcript is used downstream.The approach aligns the “load from disk” path with the existing context-sanitization path that already performs this repair, addressing provider-side strict validation errors like
unexpected tool_use_id in tool_result blocks.Confidence Score: 4/5
../agents/types.jstype import, which does not appear to exist in the repo and would break module resolution.