fix(agent): bound context compression summary stalls (salvage #49905)

[hashbender]

Summary

Salvage of NousResearch#49905 by @LeonSGP43 (rebased onto current main + a rebase-follow-up fixing a production gap the rebase exposed). Bounds context-compression summary generation with a hard timeout so a wedged summary call can't block the gateway loop indefinitely (NousResearch#49768).

The bug

call_llm forwards auxiliary.compression.timeout to the provider client as a socket/read timeout only — it does not bound the caller's wait if the network stack wedges or the SDK ignores it. The synchronous summary call in context_compressor.py was wrapped only in aux_interrupt_protection() (which suppresses interrupts — the opposite of a timeout), so compression summary generation could block the gateway loop forever. Confirmed still present on main.

The fix

@LeonSGP43's fix: run the summary call_llm in a ThreadPoolExecutor(max_workers=1) worker and stop waiting after auxiliary.compression.timeout (future.result(timeout=...), shutdown(wait=False, cancel_futures=True)). On timeout it raises, caught by the existing handler and routed into the deterministic fallback summary path — the gateway stays responsive.

Rebase + production-gap fix (this salvage)

The PR was ~592 commits behind; the compressor drifted (two cosmetic conflicts). Resolved by composing main's aux_interrupt_protection wrapper + defensive message coercion with the PR's timeout call (documented the thread-local interaction honestly: the protection guards caller-thread unpacking, the hard timeout is the stronger guarantee for the wedged-socket case — complementary, not nested).

Fixing the rebase surfaced a real production gap: the hard timeout raises a builtin TimeoutError, but _generate_summary's handler set _is_streaming_closed = _is_connection_error(e) — and _is_connection_error classifies any exception whose type name contains "Timeout" as a network close. That set _last_summary_network_failure=True, making compress() take the network-failure ABORT branch (return messages unchanged) instead of the deterministic fallback — the opposite of the PR's intent. Gated it: _is_streaming_closed = _is_connection_error(e) and not _is_timeout. Mutation-verified against the contributor's own test_compress_hard_timeout_uses_existing_fallback_path (reverting the gate fails it). Genuine streaming-closes don't carry "timeout" wording, so they're unaffected.

Review

Ran hermes-agent-dev + hermes-pr-review Phase 2c — 0 Critical. The executor pattern is correct, the abandoned-thread trade-off is bounded/documented, auxiliary.compression.timeout confirmed in DEFAULT_CONFIG, and the false-suppression edge for the _is_timeout gate was independently checked (genuine mid-stream closes are correctly preserved).

Tests

pytest tests/agent/test_context_compressor.py -q   # 142 passed (CI runs the full suite)

Supersedes NousResearch#49905. Full credit to @LeonSGP43.

---

Mirror-of: NousResearch#56295
NousResearch#56295

[tenki-reviewer]

Review Complete

Files Reviewed: 2
Findings: 1

By Severity:

• 🟡 Medium: 1

---

Adds a hard-timeout wrapper for LLM summary calls that introduces a thread safety issue: on timeout the worker thread is abandoned while call_llm continues running, creating concurrent access to unsynchronized module-level health-tracking dicts when the fallback retry path spawns a second worker.

Files Reviewed (2 files)

agent/context_compressor.py
tests/agent/test_context_compressor.py

[tenki-reviewer]

Risk: 🟡 Medium (45/100) — 1 medium finding · 121 LOC across 2 files

---

PR #295 — Context Compressor Hard-Timeout

This PR adds _call_summary_llm_with_hard_timeout in agent/context_compressor.py, wrapping LLM summary calls with a ThreadPoolExecutor and a configurable timeout. When the LLM call exceeds the timeout, the method abandons the worker thread via pool.shutdown(wait=False, cancel_futures=True) and falls back to the main model.

Thread Safety Issue

The abandoned worker thread continues running call_llm while the caller's retry path spawns a second worker in a new ThreadPoolExecutor. Both threads concurrently read/write the module-level dicts _aux_unhealthy_until and _aux_unhealthy_logged_at (in agent/auxiliary_client.py) via _mark_provider_unhealthy and _is_provider_unhealthy, with no synchronization.

Before this PR, call_llm was always called synchronously on a single thread, so the race was latent. The hard-timeout path makes concurrent access reachable.

Recommendation

Change pool.shutdown(wait=False, ...) to pool.shutdown(wait=True, ...) on the early-return success path so the worker thread is joined before control returns, eliminating the overlapping concurrent access to shared health-tracking state.

[tenki-reviewer]

🟡 Data race on _aux_unhealthy_until/_aux_unhealthy_logged_at when compression hard-timeout abandons a worker thread (bug)

The new _call_summary_llm_with_hard_timeout method (context_compressor.py:1594-1621) submits call_llm to a ThreadPoolExecutor. On timeout, pool.shutdown(wait=False, cancel_futures=True) in the finally block abandons the worker thread, which continues running call_llm. Meanwhile the caller's fallback path (context_compressor.py:2002) retries _generate_summary, which spawns a SECOND worker in a new ThreadPoolExecutor. Both threads can concurrently call call_llm, which reads/writes the module-level dicts _aux_unhealthy_until (line 2462) and _aux_unhealthy_logged_at (line 2496) via _mark_provider_unhealthy and _is_provider_unhealthy (lines 2478-2483) without any synchronization. Before this PR, call_llm was always called synchronously on a single thread, so the race was latent. The abandoned-worker pattern introduced by this PR makes concurrent access reachable.

💡 Suggestion: Change the finally block to wait for the worker thread to finish (wait=True) before returning control to the caller. This ensures the abandoned call_llm worker thread cannot overlap with a subsequent retry call, eliminating the data race on _aux_unhealthy_until and _aux_unhealthy_logged_at. On the timeout path, cancel_futures=True combined with wait=True will block briefly waiting for the already-cancelled future's thread, but the alternative is a concurrent-access bug in provider-health tracking state.

Suggested change

	finally:
	# Do not wait for a stuck network stack to unwind in the caller's
	# thread. The timed-out summary attempt is being abandoned in
	# favor of the existing deterministic compression fallback.
	pool.shutdown(wait=False, cancel_futures=True)
	finally:
	# Wait for the worker thread to finish so a subsequent retry
	# cannot overlap with an abandoned call_llm still accessing
	# shared state. The timed-out summary will still fall back.
	pool.shutdown(wait=True, cancel_futures=True)

📋 Prompt for AI Agents

In agent/context_compressor.py, inside the _call_summary_llm_with_hard_timeout method, change line 1621 from pool.shutdown(wait=False, cancel_futures=True) to pool.shutdown(wait=True, cancel_futures=True) and update the comment on lines 1618-1620 to reflect the change in behavior. The wait=True ensures the worker thread has finished before the caller's retry path spawns a new worker, eliminating the overlapping call_llm invocations that cause the data race on _aux_unhealthy_until and _aux_unhealthy_logged_at.