Conversation
chinadbo
force-pushed
the
fix/running-agents-ts-orphan-cleanup
branch
from
April 27, 2026 07:29
74691e7 to
8e6a4c8
Compare
alt-glitch
added
type/bug
chinadbo
force-pushed
the
fix/running-agents-ts-orphan-cleanup
branch
from
April 27, 2026 13:18
fa1e8d9 to
a228713
Compare
When agents were removed from _running_agents (via /stop, /new, /resume, shutdown, or hard-stop of pending agents), the corresponding entry in _running_agents_ts was not cleaned up, causing orphaned timestamps that accumulate as a memory leak and can corrupt stale-timeout checks. Add _running_agents_ts.pop() after every del _running_agents[key] and _running_agents_ts.clear() after _running_agents.clear() in shutdown.
- Replace tests that simulated cleanup inline with real _release_running_agent_state calls - Remove vacuous test_all_del_sites_have_ts_pop (trivially passes, redundant) - Add test_release_also_cleans_busy_ack_ts asserting all three dicts cleared - Remove unused AsyncMock import
The previous test called .clear() on the dicts in the test body itself, trivially passing without testing any production code path. Replace it with a source-level assertion that the shutdown block in gateway/run.py contains _running_agents_ts.clear() after _running_agents.clear() in the expected order. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chinadbo
force-pushed
the
fix/running-agents-ts-orphan-cleanup
branch
from
April 28, 2026 02:04
a228713 to
43ae049
Compare
Collaborator
|
Thanks for this careful analysis and the well-structured test plan, @chinadbo! This automated hermes-sweeper review found that the exact bug you identified was already resolved on That fix took the same approach you identified — but went a step further:
Your contribution helped confirm this was a real, worth-fixing leak — the eventual fix just landed independently via the same diagnosis path you took. Closing as implemented on main. |
teknium1
added
the
sweeper:implemented-on-main
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_running_agents_tsentries were orphaned when agents were removed from_running_agentsvia/stop,/new,/resume, shutdown, or hard-stop of pending agents_running_agents_ts.pop(key, None)after everydel _running_agents[key](6 sites) and_running_agents_ts.clear()after_running_agents.clear()in shutdown (1 site)Test plan
test_stop_command_cleans_ts— stop path cleans up timestamptest_new_command_cleans_ts— new/reset path cleans up timestamptest_resume_command_cleans_ts— resume path cleans up timestamptest_shutdown_clears_ts— shutdown clears timestampstest_all_del_sites_have_ts_pop— source-level scan ensures nodelwithoutpopwithin 5 linestest_clear_site_also_clears_ts— source-level scan ensures no.clear()without ts.clear()within 5 lines