From 82ac7e16b822582387fc2236cba251cdb33b3058 Mon Sep 17 00:00:00 2001 From: srojk34 <286497132+srojk34@users.noreply.github.com> Date: Wed, 1 Jul 2026 14:11:51 +0530 Subject: [PATCH] fix(compression): preserve network/auth abort flags across cooldown re-entry (#29559) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit compress() eagerly reset _last_summary_auth_failure and _last_summary_network_failure at the top of every call. On a second compress() during the failure cooldown, _generate_summary() returns None from the cooldown early-return WITHOUT re-asserting those flags, so the abort guard saw False and fell through to the destructive static-fallback that drops the middle window — the data-loss #29559/#25585 describe. Stop resetting them eagerly; a successful summary already clears both, so letting them persist across calls is safe and keeps the cooldown abort protection intact. Salvaged from #52056. Co-authored-by: srojk34 <286497132+srojk34@users.noreply.github.com> --- agent/context_compressor.py | 12 +++- tests/agent/test_context_compressor.py | 81 ++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/agent/context_compressor.py b/agent/context_compressor.py index 7b6dfe68e63..5a555a7f17c 100644 --- a/agent/context_compressor.py +++ b/agent/context_compressor.py @@ -2613,8 +2613,16 @@ This compaction should PRIORITISE preserving all information related to the focu self._last_aux_model_failure_error = None self._last_aux_model_failure_model = None self._last_compress_aborted = False - self._last_summary_auth_failure = False - self._last_summary_network_failure = False + # NOTE: do NOT reset _last_summary_auth_failure or + # _last_summary_network_failure here. These flags are set by + # _generate_summary() on a terminal failure and are already cleared on + # a successful summary. Resetting them eagerly defeats the cooldown + # protection: _generate_summary() returns None from the cooldown + # early-return without re-asserting these flags, so the abort guard + # below would see False and fall through to the destructive + # static-fallback — the exact data-loss #29559 describes. Letting them + # persist across compress() calls is safe because a successful summary + # always clears both. # Manual /compress (force=True) bypasses the failure cooldown so the # user can retry immediately after an auto-compress abort. Without diff --git a/tests/agent/test_context_compressor.py b/tests/agent/test_context_compressor.py index beb9c3ae39d..f87681b86ef 100644 --- a/tests/agent/test_context_compressor.py +++ b/tests/agent/test_context_compressor.py @@ -3023,3 +3023,84 @@ class TestSanitizerStripsOrphanedToolCalls: asst = next(m for m in sanitized if m.get("role") == "assistant") assert not asst.get("tool_calls") # No stub tool messages (which would have call_id != id mismatch) + + + + +class TestCooldownReentryAbort: + """Regression: a second compress() call during the failure cooldown must + still abort when the original failure was a network/auth error. + + Before the fix, compress() unconditionally reset _last_summary_network_failure + and _last_summary_auth_failure at the top of every call. When + _generate_summary() returned None from the cooldown early-return (without + re-setting the flags), the abort guard saw False and fell through to the + destructive static-fallback path — reproducing the data-loss scenario from + #29559 / #25585 that PR #51881 originally fixed. + """ + + def _msgs(self, n=12): + return [ + {"role": "user" if i % 2 == 0 else "assistant", "content": f"msg {i}"} + for i in range(n) + ] + + def test_network_failure_cooldown_reentry_still_aborts(self): + """ConnectionError → first compress aborts (PR #51881). Second + compress within the 30s cooldown must ALSO abort — not drop the + middle window via the static-fallback path.""" + with patch("agent.context_compressor.get_model_context_length", return_value=100000): + c = ContextCompressor( + model="test", + quiet_mode=True, + protect_first_n=2, + protect_last_n=2, + abort_on_summary_failure=False, + ) + msgs = self._msgs(12) + + with patch( + "agent.context_compressor.call_llm", + side_effect=ConnectionError("Connection error."), + ): + first = c.compress(msgs, current_tokens=999999, force=True) + assert first == msgs + assert c._last_compress_aborted is True + assert c._last_summary_network_failure is True + + second = c.compress(msgs, current_tokens=999999) + assert second == msgs, ( + "Second compress during cooldown must abort (preserve messages), " + "not drop the middle window via static-fallback" + ) + assert c._last_compress_aborted is True + assert c._last_summary_fallback_used is False + + def test_auth_failure_cooldown_reentry_still_aborts(self): + """Same re-entry hole for auth failures: a 401 sets the flag, cooldown + returns None, second compress must still abort.""" + err = Exception("Error code: 401 - invalid api key") + err.status_code = 401 + with patch("agent.context_compressor.get_model_context_length", return_value=100000): + c = ContextCompressor( + model="test", + quiet_mode=True, + protect_first_n=2, + protect_last_n=2, + abort_on_summary_failure=False, + ) + msgs = self._msgs(12) + + with patch("agent.context_compressor.call_llm", side_effect=err): + first = c.compress(msgs, current_tokens=999999, force=True) + assert first == msgs + assert c._last_compress_aborted is True + assert c._last_summary_auth_failure is True + + second = c.compress(msgs, current_tokens=999999) + assert second == msgs, ( + "Second compress during cooldown must abort (preserve messages), " + "not drop the middle window via static-fallback" + ) + assert c._last_compress_aborted is True + assert c._last_summary_fallback_used is False