fix(compression): let explicit interrupts cancel safely - #74449
Conversation
Related to merged #49928: that patch masks ordinary incoming-message interrupts during compression; this patch adds a distinct fence-linearized explicit-stop cancellation path across frontends. |
|
Thanks for separating explicit cancellation from ordinary incoming-message interruption. The underlying gap remains on current main: CLI Ctrl+C uses No concrete blocking implementation defect was confirmed by static review of the PR diff. This is an automated hermes-sweeper review. |
The sync compress wrapper only handled concurrent.futures.TimeoutError; KeyboardInterrupt, task cancellation, or any other exception while waiting let the host unwind while the detached worker kept full commit authority — it could later enter the commit fence and mutate durable state (in-place archival, session rotation) behind the caller's back. Wrap the whole host wait in try/finally: any exit that did not settle the worker (returned result or won the fence race) revokes future commit admission via a new lock-free CompressionCommitFence.revoke_commit_admission() (begin_commit re-checks the flag under the fence lock, so no admitted commit is ever abandoned mid-mutation). The gateway hygiene wait gets the same guarantee via a BaseException handler that revokes admission and defers helper cleanup until the worker actually returns. Reconciliation with PR #74449 (suparious): that PR routes EXPLICIT host interrupts into auxiliary-call cancellation; this change is the complementary host-side guarantee that no unwind — explicit or not — leaves an unfenced worker. The two compose (fence revocation here is the outer safety net; #74449's aux cancellation remains the fast path) rather than duplicating one another. Regressions: KeyboardInterrupt and generic-exception unwinds assert the fence is revoked WHILE the worker is still blocked pre-commit, then release the worker and prove begin_commit() is refused. PR #76354 review, blocking finding 2 / merge gate 2.
Makes interrupt-protected context compression cancellable by an explicit user or lifecycle stop, without weakening protection against ordinary incoming messages, voice interjections, or active-turn redirects. Separates explicit hard cancellation from ordinary interrupt/redirect state with a dedicated threading.Event; introduces AuxiliaryExplicitCancellation as an attempt-local frozen-cause signal; isolates the synchronous provider callback in a bounded daemon worker during protected compression; atomically linearizes Codex timeout cleanup against explicit cancellation; propagates hard cancellation through child agents and explicit stop surfaces; serializes hard-cancel admission against compression commit admission with CompressionCommitFence; aborts before session rotation or late DB commit, restores in-place transcript mutations and compressor state, and releases the heartbeat and compression lease. Based on NousResearch#74449 by @suparious. Resolved merge conflicts in agent/context_compressor.py (feasibility check + try/except) and tui_gateway/methods_session.py.
|
Salvaged via #76647 with your commit cherry-picked and authorship preserved — thank you! Your fix is exactly right: the hard-cancel Event, the daemon-isolated protected provider call, and the full cooldown/lease rollback all survived review intact (174 tests, mutation-checked, E2E-verified). The salvage rebases onto current main (your branch had drifted 51 commits and picked up some conflict-resolution damage in #76647 is armed to merge on green CI. Closing this one in its favor. |
Makes interrupt-protected context compression cancellable by an explicit user or lifecycle stop, without weakening protection against ordinary incoming messages, voice interjections, or active-turn redirects. Separates explicit hard cancellation from ordinary interrupt/redirect state with a dedicated threading.Event; introduces AuxiliaryExplicitCancellation as an attempt-local frozen-cause signal; isolates the synchronous provider callback in a bounded daemon worker during protected compression; atomically linearizes Codex timeout cleanup against explicit cancellation; propagates hard cancellation through child agents and explicit stop surfaces; serializes hard-cancel admission against compression commit admission with CompressionCommitFence; aborts before session rotation or late DB commit, restores in-place transcript mutations and compressor state, and releases the heartbeat and compression lease. Based on #74449 by @suparious. Resolved merge conflicts in agent/context_compressor.py (feasibility check + try/except) and tui_gateway/methods_session.py.
The sync compress wrapper only handled concurrent.futures.TimeoutError; KeyboardInterrupt, task cancellation, or any other exception while waiting let the host unwind while the detached worker kept full commit authority — it could later enter the commit fence and mutate durable state (in-place archival, session rotation) behind the caller's back. Wrap the whole host wait in try/finally: any exit that did not settle the worker (returned result or won the fence race) revokes future commit admission via a new lock-free CompressionCommitFence.revoke_commit_admission() (begin_commit re-checks the flag under the fence lock, so no admitted commit is ever abandoned mid-mutation). The gateway hygiene wait gets the same guarantee via a BaseException handler that revokes admission and defers helper cleanup until the worker actually returns. Reconciliation with PR #74449 (suparious): that PR routes EXPLICIT host interrupts into auxiliary-call cancellation; this change is the complementary host-side guarantee that no unwind — explicit or not — leaves an unfenced worker. The two compose (fence revocation here is the outer safety net; #74449's aux cancellation remains the fast path) rather than duplicating one another. Regressions: KeyboardInterrupt and generic-exception unwinds assert the fence is revoked WHILE the worker is still blocked pre-commit, then release the worker and prove begin_commit() is refused. PR #76354 review, blocking finding 2 / merge gate 2.
The sync compress wrapper only handled concurrent.futures.TimeoutError; KeyboardInterrupt, task cancellation, or any other exception while waiting let the host unwind while the detached worker kept full commit authority — it could later enter the commit fence and mutate durable state (in-place archival, session rotation) behind the caller's back. Wrap the whole host wait in try/finally: any exit that did not settle the worker (returned result or won the fence race) revokes future commit admission via a new lock-free CompressionCommitFence.revoke_commit_admission() (begin_commit re-checks the flag under the fence lock, so no admitted commit is ever abandoned mid-mutation). The gateway hygiene wait gets the same guarantee via a BaseException handler that revokes admission and defers helper cleanup until the worker actually returns. Reconciliation with PR #74449 (suparious): that PR routes EXPLICIT host interrupts into auxiliary-call cancellation; this change is the complementary host-side guarantee that no unwind — explicit or not — leaves an unfenced worker. The two compose (fence revocation here is the outer safety net; #74449's aux cancellation remains the fast path) rather than duplicating one another. Regressions: KeyboardInterrupt and generic-exception unwinds assert the fence is revoked WHILE the worker is still blocked pre-commit, then release the worker and prove begin_commit() is refused. PR #76354 review, blocking finding 2 / merge gate 2.
Makes interrupt-protected context compression cancellable by an explicit user or lifecycle stop, without weakening protection against ordinary incoming messages, voice interjections, or active-turn redirects. Separates explicit hard cancellation from ordinary interrupt/redirect state with a dedicated threading.Event; introduces AuxiliaryExplicitCancellation as an attempt-local frozen-cause signal; isolates the synchronous provider callback in a bounded daemon worker during protected compression; atomically linearizes Codex timeout cleanup against explicit cancellation; propagates hard cancellation through child agents and explicit stop surfaces; serializes hard-cancel admission against compression commit admission with CompressionCommitFence; aborts before session rotation or late DB commit, restores in-place transcript mutations and compressor state, and releases the heartbeat and compression lease. Based on NousResearch#74449 by @suparious. Resolved merge conflicts in agent/context_compressor.py (feasibility check + try/except) and tui_gateway/methods_session.py.
The sync compress wrapper only handled concurrent.futures.TimeoutError; KeyboardInterrupt, task cancellation, or any other exception while waiting let the host unwind while the detached worker kept full commit authority — it could later enter the commit fence and mutate durable state (in-place archival, session rotation) behind the caller's back. Wrap the whole host wait in try/finally: any exit that did not settle the worker (returned result or won the fence race) revokes future commit admission via a new lock-free CompressionCommitFence.revoke_commit_admission() (begin_commit re-checks the flag under the fence lock, so no admitted commit is ever abandoned mid-mutation). The gateway hygiene wait gets the same guarantee via a BaseException handler that revokes admission and defers helper cleanup until the worker actually returns. Reconciliation with PR NousResearch#74449 (suparious): that PR routes EXPLICIT host interrupts into auxiliary-call cancellation; this change is the complementary host-side guarantee that no unwind — explicit or not — leaves an unfenced worker. The two compose (fence revocation here is the outer safety net; NousResearch#74449's aux cancellation remains the fast path) rather than duplicating one another. Regressions: KeyboardInterrupt and generic-exception unwinds assert the fence is revoked WHILE the worker is still blocked pre-commit, then release the worker and prove begin_commit() is refused. PR NousResearch#76354 review, blocking finding 2 / merge gate 2.
The sync compress wrapper only handled concurrent.futures.TimeoutError; KeyboardInterrupt, task cancellation, or any other exception while waiting let the host unwind while the detached worker kept full commit authority — it could later enter the commit fence and mutate durable state (in-place archival, session rotation) behind the caller's back. Wrap the whole host wait in try/finally: any exit that did not settle the worker (returned result or won the fence race) revokes future commit admission via a new lock-free CompressionCommitFence.revoke_commit_admission() (begin_commit re-checks the flag under the fence lock, so no admitted commit is ever abandoned mid-mutation). The gateway hygiene wait gets the same guarantee via a BaseException handler that revokes admission and defers helper cleanup until the worker actually returns. Reconciliation with PR NousResearch#74449 (suparious): that PR routes EXPLICIT host interrupts into auxiliary-call cancellation; this change is the complementary host-side guarantee that no unwind — explicit or not — leaves an unfenced worker. The two compose (fence revocation here is the outer safety net; NousResearch#74449's aux cancellation remains the fast path) rather than duplicating one another. Regressions: KeyboardInterrupt and generic-exception unwinds assert the fence is revoked WHILE the worker is still blocked pre-commit, then release the worker and prove begin_commit() is refused. PR NousResearch#76354 review, blocking finding 2 / merge gate 2.
What does this PR do?
Makes interrupt-protected context compression cancellable by an explicit user or lifecycle stop, without weakening protection against ordinary incoming messages, voice interjections, or active-turn redirects.
The reproduced CLI failure was:
session storage could not be written.This change:
threading.Event;AuxiliaryExplicitCancellation, an attempt-local frozen-cause signal that cannot be confused with provider/plugin/OSInterruptedError;ContextVarstate plus provider progress and interrupt-protection hooks into the isolated call;hard_interrupt()with a legacyinterrupt(message=None)fallback;CompressionCommitFence, so cancellation either wins before mutation or waits for an already-admitted commit to finish;InterruptedErrorinstances;A cancelled provider daemon can outlive its owner until the request's existing provider timeout. It cannot access transcript/commit state or retain the session lease, and no shared client is closed or evicted to stop it.
Type of change
How Has This Been Tested?
Post-rebase verification against current
NousResearch/hermes-agent:main:git diff --check: passed;A repository-wide run also exercised over 50,000 tests. It exposed eight patch-related compatibility failures; all were corrected and are covered by the green affected suites above. Remaining repository-wide failures were unrelated profile opt-out/cache test-environment failures in unchanged skill/Honcho paths.
Test Configuration
uvscripts/run_tests.sh(per-file isolated subprocesses)Checklist
Documentation and downstream dependency changes are not applicable: this is internal interrupt, provider-call isolation, and compression-transaction behavior.