fix(gateway): release hygiene lock after timeout by ciabata-git · Pull Request #71569 · NousResearch/hermes-agent

ciabata-git · 2026-07-25T20:20:40Z

What does this PR do?

Fixes a gateway session-hygiene timeout race where cancellation prevented a late database commit but left the timed-out worker's durable per-session compression lock held until the synchronous auxiliary request returned. During that window the live turn could not compress and could send an oversized request.

Current main now uses progress-aware inactivity timeouts, which correctly protect slow-but-streaming summary models. Genuine inactivity and the total-ceiling path still cancel a live synchronous worker, however, and retain the same durable-lock race. This PR preserves the progress-aware wait unchanged and closes that remaining cancellation path.

The fix adds a narrow lock-setup boundary to CompressionCommitFence. Cancellation cannot win between durable lock acquisition and publication of the exact-holder cleanup callback. Once cancellation wins, the gateway releases that holder off the event loop before continuing to the live agent. Release uses a small gateway-owned executor independent of the default pool occupied by hygiene workers, so a blocked summary cannot starve its own mandatory cleanup. The worker's eventual cleanup remains idempotent and cannot release a newer holder's lock.

Related Issue

No public issue; reproduced from a production gateway incident.

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
- publish an idempotent, holder-qualified cancellation cleanup callback;
- fence durable lock acquisition through callback publication;
- stop the timed-out holder's lease refresher and release only that holder;
- preserve normal late lifecycle/client cleanup and commit-fence cancellation.
gateway/run.py
- after timeout cancellation wins, invoke the lock cleanup off-loop before continuing;
- use gateway-owned release capacity independent of the potentially saturated default executor;
- shut that release pool through the existing executor lifecycle;
- preserve the existing wait path when commit has already started.
tests/gateway/test_session_hygiene.py
- add a deterministic production-path race regression that pauses after real durable acquire but before callback publication;
- saturate executor=None work with the blocked hygiene worker and require live-agent entry plus exact lock reacquisition before releasing it;
- verify cancellation initially defers, late database mutation is fenced, cleanup runs once, and late cleanup preserves the newer holder;
- guarantee both worker barriers, handler task, executors, live-holder lock, and SessionDB unwind on every failure path.

How to Test

On current origin/main, apply only the regression-test patch and run:
.venv/bin/python -m pytest tests/gateway/test_session_hygiene.py::test_session_hygiene_timeout_continues_to_agent_and_sets_cooldown -q
It fails because try_cancel_before_commit() returns True while hook publication is still blocked.
On this branch, run:
scripts/run_tests.sh tests/gateway/test_session_hygiene.py tests/gateway/test_session_env.py tests/agent/test_compression_concurrent_fork.py tests/gateway/test_compression_concurrent_sessions.py tests/agent/test_aux_progress_streaming.py tests/agent/test_auxiliary_client.py tests/test_hermes_state_compression_locks.py tests/run_agent/test_compression_lock_defer.py -q
Expected: 504 passed, including current-main progress streaming, gateway executor lifecycle, and durable-lock coverage.
The production regression routes all default-executor work through one worker occupied by the blocked hygiene summary. It must enter the live agent and reacquire the exact session lock before that worker is released. Run the two race regressions repeatedly; the frozen candidate passed 10 consecutive iterations.
Run:
.venv/bin/python -m py_compile agent/conversation_compression.py gateway/run.py tests/gateway/test_session_hygiene.py
.venv/bin/ruff check agent/conversation_compression.py gateway/run.py tests/gateway/test_session_hygiene.py
git diff --check
A repository-wide per-file sweep was also run on the pre-progress-timeout V3 candidate. On the contributor host, the monolithic sweep exposed ambient-config/dependency/root-permission failures and three per-file timeout caps outside the changed paths. Every affected file passed when rerun under its intended isolated condition; see the verification notes below. The all-tests-pass checkbox remains deliberately unchecked because the single monolithic invocation was not green.

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: CachyOS Linux, Python 3.11

Documentation & Housekeeping

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

Screenshots / Logs

Behavioral RED on the current compression-progress base (32fd9d65cf091269709c5a6301b25aadac681aa8) with only the final regression-test patch:

assert cancel_results[0] is None
E assert True is None

The RED teardown consumed the handler and worker cleanly: no orphan future, destroyed task, secondary worker assertion, or gateway agent error.

Executor-starvation RED against the pre-V5 candidate:

AssertionError: holder-qualified release starved behind the blocked hygiene worker in the saturated default executor

Publication-base compatibility GREEN (78c06525e8e955e06a007b07b347c679f3977c3e):

=== Summary: 8 files, 504 tests passed, 0 failed ===
Full context-compressor file: 214 passed
Saturated race regressions: 10/10 consecutive runs passed

Repository-wide sweep and isolated reruns:

Discovered 2,294 test files (~43,859 tests)
Ambient config/dependency/startup rerun: 636 passed
Permission-sensitive rerun as an unprivileged UID: 371 passed, 1 skipped
Context-compressor timeout rerun: 214 passed
Web-server timeout rerun: 516 passed
Browser-hardening timeout rerun: 29 passed

Review history: V4 was rejected for default-executor starvation and incomplete RED teardown; both findings are addressed in V5. Independent immutable V5 reviews all passed with no findings or over-scope: specification/blocker resolution, concurrency/starvation/shutdown lifecycle, and code/test quality/scope.

Signed-off-by: Hermes Agent <hermes-agent@users.noreply.github.com>

alt-glitch · 2026-07-25T20:29:04Z

This was generated by AI during triage.

Related to merged #69866. This is a follow-up for the remaining durable-lock cleanup race after cancellation, with a distinct lock-publication fence and release-executor mechanism.

@ciabata-git

…ering (review F4) A host timeout previously left the timed-out worker holding the durable per-session compression lock AND refreshing its lease indefinitely, so a truly hung summary blocked every later compression attempt; and a LATE successful summary could clear the failure cooldown the host had just recorded. Transplant the lease-cancellation invariants from PR #71569 (@ciabata-git): the worker publishes an idempotent, holder-scoped release hook on the fence once it owns the durable lock (begin_lock_setup / register_cancelled_lock_release close the acquire→publish race), the refresher start is serialized against the release path, and the host invokes the hook on idle timeout, hygiene timeout, and every unwind (revoke_commit_admission now also releases). ABA safety: the SessionDB release is holder-qualified (DELETE ... WHERE holder = ?), so a stale release can never free a replacement holder's lease. State ordering: the compressor consults a fence-cancellation check BEFORE clearing the failure cooldown, so a late worker cannot undo the host's timeout cooldown; the check is installed only for the fenced call and removed in a finally. Regression implements the reviewer's exact 5-step scenario: summary blocked indefinitely → host timeout → a NEW compressor acquires the durable lock while the old summary is STILL blocked → old worker released → it cannot clear cooldown, release the new holder's lease, or publish stale state. PR #76354 review, blocking finding 4 / merge gates 4 + 5. Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>

@ciabata-git

…ering (review F4) A host timeout previously left the timed-out worker holding the durable per-session compression lock AND refreshing its lease indefinitely, so a truly hung summary blocked every later compression attempt; and a LATE successful summary could clear the failure cooldown the host had just recorded. Transplant the lease-cancellation invariants from PR #71569 (@ciabata-git): the worker publishes an idempotent, holder-scoped release hook on the fence once it owns the durable lock (begin_lock_setup / register_cancelled_lock_release close the acquire→publish race), the refresher start is serialized against the release path, and the host invokes the hook on idle timeout, hygiene timeout, and every unwind (revoke_commit_admission now also releases). ABA safety: the SessionDB release is holder-qualified (DELETE ... WHERE holder = ?), so a stale release can never free a replacement holder's lease. State ordering: the compressor consults a fence-cancellation check BEFORE clearing the failure cooldown, so a late worker cannot undo the host's timeout cooldown; the check is installed only for the fenced call and removed in a finally. Regression implements the reviewer's exact 5-step scenario: summary blocked indefinitely → host timeout → a NEW compressor acquires the durable lock while the old summary is STILL blocked → old worker released → it cannot clear cooldown, release the new holder's lease, or publish stale state. PR #76354 review, blocking finding 4 / merge gates 4 + 5. Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>

@ciabata-git

…ering (review F4) A host timeout previously left the timed-out worker holding the durable per-session compression lock AND refreshing its lease indefinitely, so a truly hung summary blocked every later compression attempt; and a LATE successful summary could clear the failure cooldown the host had just recorded. Transplant the lease-cancellation invariants from PR #71569 (@ciabata-git): the worker publishes an idempotent, holder-scoped release hook on the fence once it owns the durable lock (begin_lock_setup / register_cancelled_lock_release close the acquire→publish race), the refresher start is serialized against the release path, and the host invokes the hook on idle timeout, hygiene timeout, and every unwind (revoke_commit_admission now also releases). ABA safety: the SessionDB release is holder-qualified (DELETE ... WHERE holder = ?), so a stale release can never free a replacement holder's lease. State ordering: the compressor consults a fence-cancellation check BEFORE clearing the failure cooldown, so a late worker cannot undo the host's timeout cooldown; the check is installed only for the fenced call and removed in a finally. Regression implements the reviewer's exact 5-step scenario: summary blocked indefinitely → host timeout → a NEW compressor acquires the durable lock while the old summary is STILL blocked → old worker released → it cannot clear cooldown, release the new holder's lease, or publish stale state. PR #76354 review, blocking finding 4 / merge gates 4 + 5. Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>

@ciabata-git

…ering (review F4) A host timeout previously left the timed-out worker holding the durable per-session compression lock AND refreshing its lease indefinitely, so a truly hung summary blocked every later compression attempt; and a LATE successful summary could clear the failure cooldown the host had just recorded. Transplant the lease-cancellation invariants from PR NousResearch#71569 (@ciabata-git): the worker publishes an idempotent, holder-scoped release hook on the fence once it owns the durable lock (begin_lock_setup / register_cancelled_lock_release close the acquire→publish race), the refresher start is serialized against the release path, and the host invokes the hook on idle timeout, hygiene timeout, and every unwind (revoke_commit_admission now also releases). ABA safety: the SessionDB release is holder-qualified (DELETE ... WHERE holder = ?), so a stale release can never free a replacement holder's lease. State ordering: the compressor consults a fence-cancellation check BEFORE clearing the failure cooldown, so a late worker cannot undo the host's timeout cooldown; the check is installed only for the fenced call and removed in a finally. Regression implements the reviewer's exact 5-step scenario: summary blocked indefinitely → host timeout → a NEW compressor acquires the durable lock while the old summary is STILL blocked → old worker released → it cannot clear cooldown, release the new holder's lease, or publish stale state. PR NousResearch#76354 review, blocking finding 4 / merge gates 4 + 5. Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>

@ciabata-git

…ering (review F4) A host timeout previously left the timed-out worker holding the durable per-session compression lock AND refreshing its lease indefinitely, so a truly hung summary blocked every later compression attempt; and a LATE successful summary could clear the failure cooldown the host had just recorded. Transplant the lease-cancellation invariants from PR NousResearch#71569 (@ciabata-git): the worker publishes an idempotent, holder-scoped release hook on the fence once it owns the durable lock (begin_lock_setup / register_cancelled_lock_release close the acquire→publish race), the refresher start is serialized against the release path, and the host invokes the hook on idle timeout, hygiene timeout, and every unwind (revoke_commit_admission now also releases). ABA safety: the SessionDB release is holder-qualified (DELETE ... WHERE holder = ?), so a stale release can never free a replacement holder's lease. State ordering: the compressor consults a fence-cancellation check BEFORE clearing the failure cooldown, so a late worker cannot undo the host's timeout cooldown; the check is installed only for the fenced call and removed in a finally. Regression implements the reviewer's exact 5-step scenario: summary blocked indefinitely → host timeout → a NEW compressor acquires the durable lock while the old summary is STILL blocked → old worker released → it cannot clear cooldown, release the new holder's lease, or publish stale state. PR NousResearch#76354 review, blocking finding 4 / merge gates 4 + 5. Co-authored-by: ciabata-git <ciabata-git@users.noreply.github.com>

fix(gateway): release hygiene lock after timeout

dc987d6

Signed-off-by: Hermes Agent <hermes-agent@users.noreply.github.com>

teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026

helix4u mentioned this pull request Aug 1, 2026

feat(gateway): session activity heartbeats, stall watchdog, bounded compression waits #76354

Merged

Navigation Menu

Uh oh!

fix(gateway): release hygiene lock after timeout - #71569

fix(gateway): release hygiene lock after timeout#71569
ciabata-git wants to merge 1 commit into
NousResearch:mainfrom
ciabata-git:fix/gateway-hygiene-timeout-lock-release

ciabata-git commented Jul 25, 2026

Uh oh!

alt-glitch commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

ciabata-git commented Jul 25, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Checklist

Code

Documentation & Housekeeping

Screenshots / Logs

Uh oh!

alt-glitch commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants