Skip to content

fix: repair tool-call/result pairing on load to prevent unrecoverable 400 errors - #350

Open
rkfshakti wants to merge 3 commits into
andrewyng:mainfrom
rkfshakti:fix/repair-tool-pairing-on-load-331
Open

fix: repair tool-call/result pairing on load to prevent unrecoverable 400 errors#350
rkfshakti wants to merge 3 commits into
andrewyng:mainfrom
rkfshakti:fix/repair-tool-pairing-on-load-331

Conversation

@rkfshakti

Copy link
Copy Markdown

When a session is interrupted at the wrong moment — e.g. the user sends "continue" while a tool call is still in flight — the append-only JSONL ends up with a user message between the assistant tool_calls block and its tool result. Every provider rejects this ordering on the next message:

  • Anthropic: 400 - bad_request_error: invalid params, tool call result does not follow tool call (2013)
  • OpenAI: an assistant message with tool_calls must be followed by tool messages responding to each tool_call_id

Switching models does not help because the corruption is in the persisted history, not the provider. The session becomes permanently unrecoverable from the UI.

Root cause

ConversationStore.save() is append-only — it never rewrites history. The runtime engine already guards going forward (interrupts synthesize tool-error results so history never carries orphans), but there is no load-side counterpart for threads that predate or bypass those guards. load() passes the thread straight to the provider with no pairing check.

The fix

_repair_tool_pairing() is a static method called from ConversationStore.load() that:

  1. Moves a real tool result found later in the thread to sit immediately after its call.
  2. Synthesises a placeholder result ({"error": "tool result was lost during an interrupted turn"}) for a call with no matching tool message, so the replay is well-formed.
  3. Is idempotent — a well-formed thread passes through unchanged (early return when no repair is needed).

The repair handles multi-call blocks (multiple tool calls in one assistant message) by emitting results in call order, and correctly skips tool results that have already been moved so they do not appear twice.

Tests

tests/test_conversations_repair.py — 8 tests covering:

  • Passthrough: well-formed thread is unchanged
  • Interleaved user: user message between call and result is reordered after the result
  • Dangling call: a call with no result gets a placeholder
  • Multi-call block: multiple calls in one assistant message each get their result in order
  • Idempotent: running repair twice produces the same output
  • No tool calls: thread without tool calls is unchanged
  • Empty list: empty messages pass through
  • End-to-end load: a corrupt JSONL file is repaired on store.load()

All 8 tests pass.

Fixes #331

… 400 errors

When a turn is interrupted at the wrong moment, the append-only JSONL can
end up with a user message between an assistant tool_calls block and its
tool result. Providers reject this ordering (Anthropic 400/2013, OpenAI
'tool_call_ids did not have response messages'), making the session
permanently unrecoverable from the UI.

_repair_tool_pairing() runs in ConversationStore.load() and:
- moves a real tool result found later in the thread to sit right after its call
- synthesises a placeholder result for a call with no matching tool message
- is idempotent — well-formed threads pass through unchanged

Fixes andrewyng#331
@rkfshakti

Copy link
Copy Markdown
Author

Hi maintainers — this PR fixes #331 where a session becomes permanently unrecoverable after an interrupted turn. The append-only JSONL can end up with a user message between a tool_calls block and its tool result, which every provider rejects (Anthropic 400/2013, OpenAI tool_call_id mismatch).

The fix adds a load-side _repair_tool_pairing() that reorders results next to their calls and synthesises placeholders for dangling ones. It's idempotent — well-formed threads pass through unchanged. 8 regression tests all pass.

I'm a first-time contributor here — the CI workflow run may need approval to start. I also have active PRs on Dify (#39321, #39472, #39473, #39478) and ChromaDB (8 PRs) if that helps establish legitimacy. Thanks for taking a look!

The repair was synthesising placeholder tool results for any dangling
call, including calls in the last assistant message that are simply
pending (interrupted for approval/question). This broke durable resume
because the provider saw the placeholder and thought the tool already
ran, so resolve_inbox could not re-execute the tool.

Now trailing calls (assistant tool_calls as the last message with no
result) are left untouched — the engine will resume them. Placeholders
are only injected when the thread has moved past the call, proving it
is corrupt rather than pending.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Session permanently fails with 400/2013 after interrupt — history needs a load-side self-heal

1 participant