Skip to content

fix(agent): defer turns during compression lock contention - #49874

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/compression-lock-contention-retry
Closed

fix(agent): defer turns during compression lock contention#49874
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:fix/compression-lock-contention-retry

Conversation

@helix4u

@helix4u helix4u commented Jun 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR do?

Prevents a second same-session turn from continuing into a provider request while another path is already compressing that session.

Today, compress_context() returns the original messages unchanged when it loses the per-session compression lock. That prevents a session fork, but callers only see len(returned) == len(input), which is also the signal for genuine compression no-progress. In preflight or overflow recovery, the caller can then continue with the unchanged oversized transcript and hit the provider anyway.

This PR marks compression-lock contention explicitly on the agent and has preflight/overflow callers return a soft compression_deferred result instead of treating the unchanged messages as exhausted compression.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/conversation_compression.py
    • Adds explicit _compression_deferred_by_lock / session / holder state when the compression lock is already held.
    • Keeps the existing unchanged-message return, but no longer relies on that ambiguous signal alone.
    • Reuses one helper for compression-rotation session context updates.
  • agent/turn_context.py
    • Carries preflight lock contention through TurnContext so the turn can stop before any provider request is built.
  • agent/conversation_loop.py
    • Returns a soft compression_deferred result from preflight, 413 payload-too-large recovery, context-overflow recovery, and post-tool compression when lock contention is detected.
    • Avoids reporting the case as compression_exhausted.
  • tests/agent/test_compression_concurrent_fork.py
    • Pins the explicit defer marker on the lock-loser path.
  • tests/agent/test_turn_context.py
    • Covers preflight lock contention stopping before API context setup.

How to Test

  1. python3 -m py_compile agent/conversation_compression.py agent/turn_context.py agent/conversation_loop.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_turn_context.py tests/gateway/test_compression_concurrent_sessions.py
  2. scripts/run_tests.sh -j 4 tests/agent/test_compression_concurrent_fork.py tests/agent/test_turn_context.py tests/gateway/test_compression_concurrent_sessions.py
  3. /home/gille/.hermes/hermes-agent/venv/bin/python -m ruff check agent/conversation_compression.py agent/turn_context.py agent/conversation_loop.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_turn_context.py tests/gateway/test_compression_concurrent_sessions.py
  4. git diff --check -- agent/conversation_compression.py agent/turn_context.py agent/conversation_loop.py tests/agent/test_compression_concurrent_fork.py tests/agent/test_turn_context.py tests/gateway/test_compression_concurrent_sessions.py

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: WSL/Linux targeted test run

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

N/A

Screenshots / Logs

Targeted test output:

=== Summary: 3 files, 17 tests passed, 0 failed (100% complete) in 11.4s (4 workers) ===

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists labels Jun 21, 2026
@helix4u
helix4u marked this pull request as ready for review June 21, 2026 01:37
@alt-glitch alt-glitch mentioned this pull request Jul 11, 2026
Closed
teknium1
teknium1 reviewed Jul 14, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for isolating the ambiguous lock-loser no-op and carrying a distinct outcome through the turn result.

Problems

  • Current main added a separate pre-API compression path after this branch: 90f84144e calls _compress_context() at agent/conversation_loop.py:1034 and unconditionally continues at line 1063. It is not covered by this PR's diff, so a lock-loser there would still continue toward a provider request.
  • The new preflight unit test is useful, but it does not cover that current-main pre-API path or prove that a deferred lock result prevents the provider call.

Suggested changes

  • During salvage, apply the same soft-deferral check immediately after agent/conversation_loop.py:1034.
  • Add provider-mock regressions for both preflight and pre-API lock contention; assert no request is dispatched and the result is compression_deferred, not compression_exhausted.

Automated hermes-sweeper review.

Comment thread agent/conversation_loop.py
@@ -560,6 +590,14 @@ def run_conversation(
_plugin_user_context = _ctx.plugin_user_context

@teknium1 teknium1 Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please carry this same deferral check to the current-main pre-API compaction path introduced by 90f84144e: it calls _compress_context() at agent/conversation_loop.py:1034 and otherwise continues toward request construction. Add a regression that pre-holds the lock and asserts no provider request is dispatched.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/compression Context compression and continuation sessions labels Jul 14, 2026
@teknium1 teknium1 mentioned this pull request Jul 23, 2026
Merged
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
@helix4u @teknium1
fix(agent): defer turns during compression lock contention instead of…
424c485 
… exhausting

A lock-loser compression pass returns its input unchanged, which the
automatic compression sites misread as 'cannot compress further': the
preflight loop armed the insufficient-progress blocker, the pre-API gate
burned a shared attempt, and a lock-contended 413/overflow retried into
the attempt cap and returned compression_exhausted — which the gateway
answers with a full session auto-reset (#9893/#35809). A temporary
concurrent-compression defer wiped the session.

Consume the landed #69870 lock-skip signal on every automatic path
(preflight in turn_context, pre-API pressure gate, 413 handler, overflow
handler, post-tool compaction): when a pass no-ops AND the type-pinned
lock-skip flag is set, refund the attempt (never count it toward the cap
or the insufficient-progress blocker), and when the turn cannot proceed
(provider already proved the request does not fit) end it with a soft
compression_deferred result — distinct from compression_exhausted — so
the gateway keeps the session intact and the next message retries after
the concurrent compressor finishes.

The new compression_skipped_due_to_lock() reader is type-pinned
(is True or isinstance(str)) per the MagicMock auto-attribute rule, and
compress_context() now also clears the signal at the very top of every
attempt (per-attempt state rule, #58629/#69853) so a stale value can
never make a later breaker/codex no-op look like lock contention.

Salvaged from PR #49874; rebuilt on main's #69870
_compression_skipped_due_to_lock signal instead of the PR's parallel
_compression_deferred_by_lock triple.
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
fix(gateway): retry-next-message semantics for compression_deferred +…
f249889 
… regression suite

Gateway half of the #49874 salvage: pass compression_deferred through
both _run_agent_inner result dicts and guard the compression-exhausted
auto-reset block with it — a lock-contended defer keeps the session
intact (the concurrent compressor is actively shrinking it) instead of
wiping it via reset_session.

Regression tests:
- tests/run_agent/test_compression_lock_defer.py — provider-mock 413 and
  400-overflow turns whose compression pass lost the lock end as
  compression_deferred (failed=False, no compression_exhausted); flag
  unset keeps the terminal exhaustion path byte-identical; type-pin
  tests vs MagicMock agents and junk flag values; cap=1 e2e proving the
  refunded pre-API defer leaves the budget for the provider-proven
  413 retry.
- tests/agent/test_preflight_lock_defer.py — a lock-skipped preflight
  pass stops the loop WITHOUT arming preflight_compression_blocked;
  plain no-op still arms it; MagicMock junk does not defer.
- tests/gateway/test_compression_deferred_soft_result.py — AST pin that
  the deferred branch guards the auto-reset chain and performs no
  session mutation (mirrors test_35809_auto_reset_clean_context.py).
teknium1 pushed a commit that referenced this pull request Jul 23, 2026
@helix4u @teknium1
fix(agent): defer turns during compression lock contention instead of…
056a40a 
… exhausting

A lock-loser compression pass returns its input unchanged, which the
automatic compression sites misread as 'cannot compress further': the
preflight loop armed the insufficient-progress blocker, the pre-API gate
burned a shared attempt, and a lock-contended 413/overflow retried into
the attempt cap and returned compression_exhausted — which the gateway
answers with a full session auto-reset (#9893/#35809). A temporary
concurrent-compression defer wiped the session.

Consume the landed #69870 lock-skip signal on every automatic path
(preflight in turn_context, pre-API pressure gate, 413 handler, overflow
handler, post-tool compaction): when a pass no-ops AND the type-pinned
lock-skip flag is set, refund the attempt (never count it toward the cap
or the insufficient-progress blocker), and when the turn cannot proceed
(provider already proved the request does not fit) end it with a soft
compression_deferred result — distinct from compression_exhausted — so
the gateway keeps the session intact and the next message retries after
the concurrent compressor finishes.

The new compression_skipped_due_to_lock() reader is type-pinned
(is True or isinstance(str)) per the MagicMock auto-attribute rule, and
compress_context() now also clears the signal at the very top of every
attempt (per-attempt state rule, #58629/#69853) so a stale value can
never make a later breaker/codex no-op look like lock contention.

Salvaged from PR #49874; rebuilt on main's #69870
_compression_skipped_due_to_lock signal instead of the PR's parallel
_compression_deferred_by_lock triple.
teknium1 added a commit that referenced this pull request Jul 23, 2026
@teknium1
fix(gateway): retry-next-message semantics for compression_deferred +…
eebc228 
… regression suite

Gateway half of the #49874 salvage: pass compression_deferred through
both _run_agent_inner result dicts and guard the compression-exhausted
auto-reset block with it — a lock-contended defer keeps the session
intact (the concurrent compressor is actively shrinking it) instead of
wiping it via reset_session.

Regression tests:
- tests/run_agent/test_compression_lock_defer.py — provider-mock 413 and
  400-overflow turns whose compression pass lost the lock end as
  compression_deferred (failed=False, no compression_exhausted); flag
  unset keeps the terminal exhaustion path byte-identical; type-pin
  tests vs MagicMock agents and junk flag values; cap=1 e2e proving the
  refunded pre-API defer leaves the budget for the provider-proven
  413 retry.
- tests/agent/test_preflight_lock_defer.py — a lock-skipped preflight
  pass stops the loop WITHOUT arming preflight_compression_blocked;
  plain no-op still arms it; MagicMock junk does not defer.
- tests/gateway/test_compression_deferred_soft_result.py — AST pin that
  the deferred branch guards the auto-reset chain and performs no
  session mutation (mirrors test_35809_auto_reset_clean_context.py).
@teknium1

teknium1 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Merged via salvage PR #70285 — thanks @helix4u for identifying the exhaustion-misclassification bug: a lock-contended compression skip could burn the attempt cap on 413 and return compression_exhausted, triggering the gateway's auto-reset and wiping the session over a transient lock race. Your caller-side soft-defer design is what landed; it was rebuilt on the _compression_skipped_due_to_lock signal that merged after your branch (superseding the PR's setter half), so this closes in favor of the salvage with the diagnosis and design credited to you in the PR body.

randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@helix4u @teknium1
fix(agent): defer turns during compression lock contention instead of…
cceff4c 
… exhausting

A lock-loser compression pass returns its input unchanged, which the
automatic compression sites misread as 'cannot compress further': the
preflight loop armed the insufficient-progress blocker, the pre-API gate
burned a shared attempt, and a lock-contended 413/overflow retried into
the attempt cap and returned compression_exhausted — which the gateway
answers with a full session auto-reset (NousResearch#9893/NousResearch#35809). A temporary
concurrent-compression defer wiped the session.

Consume the landed NousResearch#69870 lock-skip signal on every automatic path
(preflight in turn_context, pre-API pressure gate, 413 handler, overflow
handler, post-tool compaction): when a pass no-ops AND the type-pinned
lock-skip flag is set, refund the attempt (never count it toward the cap
or the insufficient-progress blocker), and when the turn cannot proceed
(provider already proved the request does not fit) end it with a soft
compression_deferred result — distinct from compression_exhausted — so
the gateway keeps the session intact and the next message retries after
the concurrent compressor finishes.

The new compression_skipped_due_to_lock() reader is type-pinned
(is True or isinstance(str)) per the MagicMock auto-attribute rule, and
compress_context() now also clears the signal at the very top of every
attempt (per-attempt state rule, NousResearch#58629/NousResearch#69853) so a stale value can
never make a later breaker/codex no-op look like lock contention.

Salvaged from PR NousResearch#49874; rebuilt on main's NousResearch#69870
_compression_skipped_due_to_lock signal instead of the PR's parallel
_compression_deferred_by_lock triple.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
@teknium1
fix(gateway): retry-next-message semantics for compression_deferred +…
ab4dca6 
… regression suite

Gateway half of the NousResearch#49874 salvage: pass compression_deferred through
both _run_agent_inner result dicts and guard the compression-exhausted
auto-reset block with it — a lock-contended defer keeps the session
intact (the concurrent compressor is actively shrinking it) instead of
wiping it via reset_session.

Regression tests:
- tests/run_agent/test_compression_lock_defer.py — provider-mock 413 and
  400-overflow turns whose compression pass lost the lock end as
  compression_deferred (failed=False, no compression_exhausted); flag
  unset keeps the terminal exhaustion path byte-identical; type-pin
  tests vs MagicMock agents and junk flag values; cap=1 e2e proving the
  refunded pre-API defer leaves the budget for the provider-proven
  413 retry.
- tests/agent/test_preflight_lock_defer.py — a lock-skipped preflight
  pass stops the loop WITHOUT arming preflight_compression_blocked;
  plain no-op still arms it; MagicMock junk does not defer.
- tests/gateway/test_compression_deferred_soft_result.py — AST pin that
  the deferred branch guards the auto-reset chain and performs no
  session mutation (mirrors test_35809_auto_reset_clean_context.py).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

area/compression Context compression and continuation sessions comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@helix4u @teknium1 @alt-glitch