fix(agent): make tool error messages ephemeral to prevent HTTP 400 replay loop

[vynxevainglory-ai]

 1|     1|## What does this PR do?
 2|     2|
 3|     3|When a tool call returns an error (exception, non-zero exit, guardrail block, user interrupt), the error result was persisted verbatim to the session DB and replayed on every subsequent turn. If the error content contained characters violating the LLM provider's API contract (oversized JSON, special chars, unknown dict keys), every follow-up message triggered HTTP 400 — an unrecoverable loop requiring `/new`.
 4|     4|
 5|     5|This PR tags error tool messages with `_is_error: True` at source (tool_executor.py), skips them during session DB persistence (run_agent.py), strips the flag from API message copies for strict providers (conversation_loop.py, chat_completion_helpers.py), and provides backwards-compatible load-time filters for pre-fix sessions (gateway + CLI resume paths).
 6|     6|
 7|     7|## Related Issue
 8|     8|
 9|     9|Related to #27033
10|    10|
11|    11|## Type of Change
12|    12|
13|    13|- [x] 🐛 Bug fix (non-breaking change that fixes an issue)
14|    14|
15|    15|## Changes Made
16|    16|
17|    17|- `agent/tool_executor.py` — Tag error tool messages with `_is_error: True` at all 4 append sites (concurrent preflight interrupt, concurrent post-execution, sequential post-execution, sequential interrupt skip)
18|    18|- `run_agent.py` — Skip `_is_error` messages in `_flush_messages_to_session_db` so errors live in-memory for the current turn only
19|    19|- `agent/conversation_loop.py` — Strip `_is_error` from API message copy before serialization (alongside `_thinking_prefill`, `finish_reason`, `reasoning`)
20|    20|- `agent/chat_completion_helpers.py` — Strip `_is_error` in max-iterations API message path
21|    21|- `agent/tool_dispatch_helpers.py` — Extract shared `is_tool_error_message()` helper for content-based backwards compat detection
22|    22|- `cli.py` — Load-time filter via `_is_ephemeral_tool_error` in both resume paths
23|    23|- `gateway/run.py` — Load-time filter via `is_tool_error_message` import in `_build_gateway_agent_history`
24|    24|- `tests/run_agent/test_27033_tool_error_ephemeral.py` — 25 tests covering all layers
25|    25|
26|    26|## How to Test
27|    27|
28|    28|1. Run the regression tests: `pytest tests/run_agent/test_27033_tool_error_ephemeral.py -v`
29|    29|2. Run broader regression: `pytest tests/ -k "flush or dedup or tool_error or strip or ephemeral" -v`
30|    30|3. All 25 new tests pass. 978/978 related tests pass, 0 regressions.
31|    31|
32|    32|## Serialization Audit
33|    33|
34|    34|- [x] `_is_error` stripped in `conversation_loop.py` (main API path)
35|    35|- [x] `_is_error` stripped in `chat_completion_helpers.py` (max-iterations path)
36|    36|- [x] Checked all three serialization paths: `conversation_loop.py`, `chat_completion_helpers.py`, `gateway/run.py`
37|    37|- [x] Safe for strict providers (Mistral, Fireworks) — `msg.copy()` preserves original for flush filter
38|    38|
39|    39|## Backwards Compatibility
40|    40|
41|    41|- [x] Old format (pre-fix sessions): content-based pattern matching detects persisted error messages by prefix (`Error executing tool `, `[Tool execution cancelled`, `[Tool execution skipped`)
42|    42|- [x] New format (post-fix sessions): explicit `_is_error` flag
43|    43|- [x] Load-time filter added for pre-fix data in both CLI resume paths and gateway replay
44|    44|
45|    45|## Test Coverage
46|    46|
47|    47|| Test | What it verifies |
48|    48||------|-----------------|
49|    49|| `test_make_tool_result_message_has_no_is_error_by_default` | Messages from `make_tool_result_message` don't carry `_is_error` by default |
50|    50|| `TestIsEphemeralToolError::test_detects_error_tool_messages_by_content` | CLI filter catches all 4 error content patterns |
51|    51|| `TestIsEphemeralToolError::test_ignores_non_error_messages` | CLI filter doesn't false-positive on normal content |
52|    52|| `TestIsEphemeralToolError::test_detects_is_error_flag` | Explicit `_is_error` flag detected |
53|    53|| `TestIsEphemeralToolError::test_ignores_non_string_content` | Non-string content handled safely |
54|    54|| `TestFlushSkipsErrorMessages::test_error_messages_are_skipped` | `_is_error` messages not flushed to session DB |
55|    55|| `TestFlushSkipsErrorMessages::test_mixed_batch_preserves_order` | Mixed batch preserves order for non-error messages |
56|    56|| `test_last_flushed_db_idx_advances_past_skipped_messages` | Dedup tracking advances past skipped errors |
57|    57|| `test_cli_resume_filter_removes_error_tool_messages` | CLI resume path strips error messages |
58|    58|| `test_gateway_stale_tool_error_helper` | Gateway replay uses shared `is_tool_error_message` |
59|    59|| `test_is_error_stripped_from_api_copy` | `_is_error` stripped from API copy, preserved in original |
60|| `test_error_format_prefixes_match_tool_executor` | Content-based filter catches all tool_executor.py error formats |
61|    60|| Full suite: 25/25 pass, 0 regressions | |
62|    61|
63|    62|## Checklist
64|    63|
65|    64|### Code
66|    65|
67|    66|- [x] I've read the [Contributing Guide](https://github.com/NousResearch/hermes-agent/blob/main/CONTRIBUTING.md)
68|    67|- [x] My commit messages follow [Conventional Commits](https://www.conventionalcommits.org/) (`fix(scope):`)
69|    68|- [x] I searched for [existing PRs](https://github.com/NousResearch/hermes-agent/pulls) to make sure this isn't a duplicate
70|    69|- [x] My PR contains **only** changes related to this fix (no unrelated commits)
71|    70|- [x] I've run `pytest tests/ -q` and all tests pass
72|    71|- [x] I've added tests for my changes
73|    72|- [x] I've tested on my platform: Linux (WSL2)
74|    73|
75|    74|### Documentation & Housekeeping
76|    75|
77|    76|- [x] I've updated relevant documentation — or N/A
78|    77|- [x] I've updated `cli-config.yaml.example` if I added/changed config keys — or N/A
79|    78|- [x] I've updated `CONTRIBUTING.md` or `AGENTS.md` if I changed architecture or workflows — or N/A
80|    79|- [x] I've considered cross-platform impact — changes are pure Python dict manipulation, no OS-specific code
81|    80|- [x] I've updated tool descriptions/schemas if I changed tool behavior — or N/A
82|    81|
83|    82|## Merge Confidence
84|    83|
85|    84|**HIGH** — The fix is minimal (tag at source, skip at flush, strip at wire), backwards compatible (content-based fallback for pre-fix sessions), well-tested (25 tests covering all layers including edge cases), and doesn't touch any unrelated code paths.
86|    85|
87|    86|## Relationship to #18650
88|    87|
89|    88|PR #18650 (still open, merge conflicts) addresses a *structural* variant of the same loop family: tool messages with null/empty `tool_call_id` produced by compression, plus a runtime retry hook. Its sanitizer strips *orphaned* tool messages (no matching assistant call) but does not touch messages with valid `tool_call_id` — which is the common case for #27033.
90|    89|
91|    90|This PR complements #18650 by tackling the *content persistence* side: preventing error tool results from ever reaching the session DB, regardless of whether the assistant message is still present. Neither PR makes the other redundant.
92|    91|

[vynxevainglory-ai]

Complementary to #18650, not competing. #18650 fixes compression-induced tool_call_id corruption. This fixes error-content contamination at source — valid tool_call_id but malformed content. Both produce the same 400 loop from different triggers. Happy to rebase on top of #18650 if you want both, or close if #18650's recovery covers the gap sufficiently.

[teknium1]

Thanks for the thorough work here, @vynxevainglory-ai — the serialization audit across all three API paths and the 25 regression tests are exactly the kind of rigor we want. We're going to pass on this one, though, for two reasons.

1. The loop in #27033 is no longer reproducible on current main. That issue was filed against v0.10.0. Since then the malformed-tool-content 400 cases it describes are handled by layered defenses: _sanitize_tool_error strips framing tokens and caps length at the source; _repair_tool_call_arguments fixes malformed tool-call arguments; surrogate/non-ASCII 400s are sanitized-and-retried in conversation_loop.py; orphaned tool blocks are stripped by _strip_orphaned_tool_blocks; and a generic content-400 classifies as a non-retryable format_error with should_fallback=True (deterministic fallback, not an infinite /new-only loop).

2. The fix direction is too broad. Tagging every tool error with _is_error and dropping it on persistence / filtering it on resume removes legitimate conversation context. Tool errors aren't just noise — the model uses them to change course ("the build failed, so I'll try X"). Making all of them ephemeral means a resumed session loses why the agent did what it did, which is a behavior change well beyond the reported bug, and the actual recurring-400 cases are already neutralized at the sanitize/repair/fallback layers above rather than at persistence.

The genuinely-live remnant of this family is the stale-tool_call_id variant tracked at #16472 — scoped specifically to IDs that go orphaned across sessions, not to wholesale error-result suppression. If you'd like to take a focused pass at that one, it'd be welcome.

Closing #27033 as substantially-addressed alongside this.