fix(agent): stop the send-path repair from rewriting persisted history

[0xGr1mm]

What does this PR do?

_canonicalize_api_tool_calls promises copy-on-write in its own docstring — "the persisted history is untouched" — and the call site repeats it: "Operates on api_messages (the API copy) so the original conversation history in messages is untouched."

The canonicalize branch keeps that promise. The repair branch does not:

try:
    tc = {**tc, "function": {**tc["function"], "arguments": _canonicalize_tool_call_arguments(...)}}
except Exception:
    tc["function"]["arguments"] = _repair_tool_call_arguments(...)   # writes through

api_messages is built with msg.copy() — a shallow per-message copy — so every tool_calls entry is the same dict object the persisted history holds. Assigning into tc["function"] therefore rewrites the stored turn. The sibling loop two lines above only touches am["content"], one level deep, which is why the aliasing never surfaced there.

On the unrepairable path _repair_tool_call_arguments returns "{}", so that write replaces the model's real arguments with an empty object in the transcript.

Why it matters

This is the mechanism behind the silent data loss reported in #80498. A stream that dies mid write_file loses the file content it had already streamed, leaving only a warning:

WARNING agent.message_sanitization: Unrepairable tool_call arguments for write_file —
replaced with empty object (was: {"content": "# 骨架-第25章\n> 承接...)

Reproduced against the shipped function with the send path's own aliasing shape:

BEFORE history args: '{"content": "# 骨架-第25章\n> 承接...'
AFTER  history args: '{}'

persisted history mutated? True
original content destroyed? True

The fix

Mirror the canonicalize branch: build a new tool-call dict instead of assigning into the shared one. The API copy still carries "{}" — never shipping broken JSON is the repair's entire purpose — but the history keeps what the model actually sent, so the transcript, session persistence and any later retry still have it.

Why this survived so long

The in-place write is older than the memo refactor, which preserved it deliberately for byte-parity. The existing test_history_not_mutated asserts exactly this invariant, but restricts itself to valid arguments, and its docstring records the gap:

(Malformed args take the in-place repair path — pre-existing behavior, identical in both implementations; see parity tests.)

So a test file whose header already claims "the persisted history is never mutated (copy-on-write preserved)" stayed green straight through the bug.

Related Issue

Refs #80498 (P1, type/bug, comp/agent, tool/file) — this PR fixes the history-corruption half of that report. Searched open and merged PRs first, per CONTRIBUTING's search-first section:

gh search prs --repo NousResearch/hermes-agent "tool_call arguments in:title"    --limit 30
gh search prs --repo NousResearch/hermes-agent "Unrepairable tool_call"          --limit 30
gh search prs --repo NousResearch/hermes-agent "mid-tool-call stream drop"       --limit 30
gh search prs --repo NousResearch/hermes-agent "tool call arguments truncated"   --limit 30
gh search prs --repo NousResearch/hermes-agent "stream drop in:title"            --limit 30
gh search prs --repo NousResearch/hermes-agent "80498"                           --limit 10

Nothing addresses this. The nearest open PRs are all on the repair side and touch different code: #16505 repairs args on the invalid-JSON recovery write site in run_agent.py, #79333 recovers wrapped args in message_sanitization.py, #74149 only widens that module's log line. #42314 (merged, P1) added the mid-tool-call-drop detection quoted in the issue; it is upstream of this and unchanged here.

Type of Change

Bug fix (non-breaking).

Changes Made

• agent/conversation_loop.py — the repair branch of _canonicalize_api_tool_calls is now copy-on-write, matching the canonicalize branch; comment records why the aliasing exists.
• tests/agent/test_canon_args_memo_parity.py — 4 tests in a new class covering the repair path the existing history test deliberately excluded.

How to Test

pytest tests/agent/test_canon_args_memo_parity.py -q

13 passed.

Reverting only agent/conversation_loop.py and rerunning:

FAILED TestUnrepairableArgsAreNotWrittenBackToHistory::test_history_keeps_the_original_arguments
FAILED TestUnrepairableArgsAreNotWrittenBackToHistory::test_valid_calls_alongside_a_broken_one_are_untouched
FAILED TestUnrepairableArgsAreNotWrittenBackToHistory::test_repeated_sends_do_not_accumulate_damage
3 failed, 10 passed

with

E  AssertionError: the send-path canonicalizer rewrote the persisted history
E  assert '{}' == '{"content": ...one\nline two'

The fourth new test (test_send_copy_is_still_repaired) passes both before and after by design — it pins that the API copy is still repaired, so this change is only about the aliasing.

The 9 pre-existing tests, including the byte-parity and complexity proofs, pass unchanged: the difference is only observable when the history list is separate from the send copy, which is the shape production uses and the old parity harness did not.

tests/agent/ + tests/run_agent/, this branch vs main, same environment, run serially:

main:   171 failed, 5072 passed, 18 skipped, 2 deselected
branch: 171 failed, 5076 passed, 18 skipped, 2 deselected

Failing sets are identical — zero added, zero removed. Those 171 are pre-existing in a non-hermetic single-process run; CI's per-file isolation via run_tests_parallel.py is the supported path.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(agent): …)
• I searched for existing PRs to make sure this isn't a duplicate (queries above)
• My PR contains only changes related to this fix
• I've run the affected suites and all tests pass
• I've added tests for my changes
• I've tested on my platform: macOS 15 (Darwin 25.5), Python 3.11

Documentation & Housekeeping

• I've updated relevant documentation — the restored invariant is the one both the docstring and the call-site comment already state; the new comment explains the aliasing that made it easy to break
• cli-config.yaml.example — N/A, no config keys
• CONTRIBUTING.md / AGENTS.md — N/A
• Cross-platform impact — N/A, dict handling only
• Tool descriptions/schemas — N/A

Notes for the reviewer

Deliberately scoped to the aliasing. #80498 also asks that a truncated call not be executed with empty arguments and that the model be told it failed; that is a separate decision in the stream/tool-dispatch path (_build_partial_stream_stub already drops the calls when finish_reason is None, so the remaining execution path is narrower than the report implies). I did not fold it in — it changes turn control flow, where this change only stops the transcript from being overwritten. Happy to follow up if you want that half too.

One consequence worth naming: broken arguments now stay broken in history, so each send re-runs the failed json.loads and repair for that message instead of repairing it once. The memo only caches successes, so this was already true for every malformed argument that repaired to something other than "{}" — it is one failed parse per affected message per iteration, against a transcript that no longer loses data.

[kshitijk4poor]

Merged via #80963 with your commit cherry-picked and authorship preserved (rebase-merge — fix(agent): stop the send-path repair from rewriting persisted history is on main under your name, tests included).

Your diagnosis was exactly right — and it generalized: probing every send-path transform against the shallow-copy aliasing shape showed the surrogate and non-ASCII sanitizers leaking into stored multimodal parts and reasoning fields through the same mechanism your repair-branch finding exposed. The salvage keeps your branch-level copy-on-write as defense in depth and adds a structural clone at the api_messages build chokepoint so the whole class is closed by construction, plus full-bytes WARNING logging at the pre-send transcript sanitizer (the pass the #80498 incident actually hit first). Your four regression tests pin the branch layer independently of the chokepoint — good layering that survives either being refactored.

Thanks for the excellent find and the clean, well-tested fix!