fix(gateway): bound hygiene compression failures - #53380
Conversation
Related: #49905 bounds the agent context-compression summary stall; this PR bounds the gateway pre-agent hygiene compression path. Same compression-stall family, different code path — flagging the cluster so a maintainer can decide whether to consolidate. |
|
Thanks for flagging. Agreed: this is the same compression-stall family as #49905, but it intentionally covers the gateway pre-agent hygiene path rather than the agent's in-loop context-compressor summary path. I'm happy to rebase/consolidate if maintainers prefer that both timeout/fallback paths land together, but keeping this PR scoped to gateway hygiene avoids changing the agent compression wrapper while still preventing inbound gateway messages from repeatedly blocking on a slow/depleted auxiliary compression backend. |
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Comment (core gateway change)
This PR adds 293 lines (22KB diff) to gateway/run.py, introducing timeout and cooldown settings for hygiene compression failures. The changes are well-documented and follow the existing config parsing pattern.
However, this modifies core gateway behavior (compression failure handling) and the 22KB diff size includes significant context. Key considerations:
- Timeout and cooldown settings affect all conversations
- Default values (30s timeout, 300s cooldown) need validation against production behavior
- Config parsing follows existing patterns but touches critical path
The implementation is clean, but the gateway is a critical component. Recommend human review of the timeout/cooldown defaults and their interaction with existing compression logic.
Reviewed by Hermes Agent
|
Small consolidation note after related compaction fixes landed elsewhere: this PR still appears to cover a distinct path. The merged Codex/max-output and in-loop compaction fixes address agent-side recovery/flush behavior. This PR is about the gateway pre-agent hygiene compression path, where a slow or broken auxiliary compression backend can block before the agent turn is even reached. So the useful review question is whether Hermes wants a bounded timeout/cooldown for that gateway hygiene path, independent of the agent-context compression recovery fixes. |
72a47e0 to
e43d4b5
Compare
|
Rebased this branch onto current main and resolved the gateway hygiene conflict. The conflict resolution keeps the PR's timeout behavior, but uses the current main off-loop cleanup path when the hygiene compression worker did not time out. Validation run locally:
|
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the distinct gateway pre-agent hygiene stall path; current main still awaits this compression before _run_agent at gateway/run.py:11316-11322.
Problems
- The timeout path starts executor work at
gateway/run.py:10991and then intentionally skips the only cleanup atgateway/run.py:11173-11176. The current cleanup routine shuts down the memory provider and closes tool/client resources (gateway/run.py:6037-6076), so a timed-out worker that eventually returns leaks its temporary agent. - Current main now binds hygiene compaction to the live SessionDB (
gateway/run.py:11278-11309) and starts_run_agentafter hygiene failure handling (gateway/run.py:11632-11650). Salvage must not let a timed-out worker continue mutating that session concurrently with the new turn.
Suggested changes
- Use a lifecycle boundary that stops/fences timed-out compression before the live turn proceeds, and ensure cleanup happens after worker completion.
- Add a SessionDB-bound timeout regression covering both no post-timeout compaction and eventual resource cleanup.
Automated hermes-sweeper review.
| @@ -10953,13 +10987,53 @@ async def _handle_message_with_agent(self, event, source, _quick_key: str, run_g | |||
| _hyg_agent._print_fn = lambda *a, **kw: None | |||
|
|
|||
| loop = asyncio.get_running_loop() | |||
| _compressed, _ = await loop.run_in_executor( | |||
| _hyg_timed_out = False | |||
| _hyg_future = loop.run_in_executor( | |||
There was a problem hiding this comment.
wait_for() bounds the await, not this executor thread. The timeout path later skips cleanup entirely, so a worker that eventually returns leaks its temporary AIAgent; once salvaged onto current main, that worker can also race the SessionDB-bound hygiene compactor. Please fence/terminate completion before proceeding to the live turn and arrange cleanup after it is safe.
e43d4b5 to
95adbe5
Compare
|
Merged via #69866 (commit 2e9765b). Salvaged with worker fencing added: bounded await + cancellation-token no-op for late workers + temp-agent cleanup + lock-reacquire verification. Note: the commit was authored by @kshitijk4poor and lands with his authorship; PR-submission credit to you in the merged body. |
Summary
compression.hygiene_timeout_secondsandcompression.hygiene_failure_cooldown_secondsknobsWhy
Long-running gateway sessions can become oversized and trigger hygiene compression before handling an inbound message. If the auxiliary compression backend is depleted, misconfigured, or slow, the gateway can appear stuck by repeatedly blocking on that pre-agent recovery path.
Tests
python -m pytest tests/gateway/test_session_hygiene.py -qpython -m pytest tests/gateway/test_session_hygiene.py tests/gateway/test_compress_command.py::test_compress_command_passes_session_db_and_persists_rotated_session tests/agent/test_compression_rotation_state.py tests/agent/test_compression_concurrent_fork.py -qpython -m py_compile gateway/run.py hermes_cli/config.pygit diff --check