Skip to content

fix: stop PulseAgent blocking the hypercode-core event loop #311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
welshDog merged 1 commit into from
Jul 10, 2026
Merged

fix: stop PulseAgent blocking the hypercode-core event loop #311

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions backend/app/agents/pulse.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,18 @@
import logging
import requests
import httpx
from app.agents.brain import brain

logger = logging.getLogger(__name__)

# Built once, at import. Constructing an AsyncClient builds an SSL context and
# loads the CA bundle — hundreds of ms of *synchronous* work. Doing that inside
# process() stalls the loop harder than the sync requests.get it replaced.
# Measured in-container, worst loop stall per call:
# requests.get (sync) 53 ms
# AsyncClient built per call 284 ms <- worse than the bug
# shared AsyncClient (this) 8 ms
_http = httpx.AsyncClient(timeout=5.0)

@coderabbitai coderabbitai Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Module-level AsyncClient is never closed — connection pool leaks on shutdown.

The shared _http client is intentionally long-lived, but there is no aclose() call anywhere. On process restart or graceful shutdown, the underlying connection pool and SSL context are not disposed, which can leak sockets and file descriptors.

Consider wiring an aclose() into the application's lifespan/shutdown handler, or use a pattern that ties the client lifecycle to the app:

♻️ Suggested lifecycle management 
 _http = httpx.AsyncClient(timeout=5.0)
+
+async def close_http_client() -> None:
+    """Call during application shutdown."""
+    await _http.aclose()

Then invoke close_http_client() from the app's shutdown signal handler (e.g., FastAPI lifespan or equivalent).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_http = httpx.AsyncClient(timeout=5.0)
_http = httpx.AsyncClient(timeout=5.0)
async def close_http_client() -> None:
"""Call during application shutdown."""
await _http.aclose()
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/agents/pulse.py` at line 14, The shared module-level AsyncClient
in pulse.py is never closed, so its connection pool can leak on shutdown. Add a
dedicated close helper for the _http client (for example, a close_http_client
function near the existing client setup) that awaits _http.aclose(), and make
sure the application shutdown path or lifespan handler invokes it. Keep the
client reuse pattern, but wire its lifecycle to startup/shutdown so the
singleton client is disposed cleanly.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Is it safe to create httpx.AsyncClient at module import time (outside an event loop) and use it later inside an async context in httpx 0.28.1?

💡 Result:

It is generally unsafe and not recommended to create an httpx.AsyncClient at module import time. While initializing the AsyncClient object itself does not immediately require an active event loop, the client's internal mechanisms (such as connection pooling and transport initialization) are designed to operate within an asynchronous context. Creating it at the module level introduces several risks: 1. Lifecycle Management: The AsyncClient is intended to be used as a context manager (using async with) or explicitly closed using await client.aclose() to ensure resources like connection pools are properly released [1][2]. Module-level objects persist for the lifetime of the application, making it difficult to ensure they are shut down gracefully, which can lead to resource leaks and warnings about unclosed clients [3]. 2. Event Loop Binding: While newer versions of httpx have improved compatibility, many asynchronous libraries in Python bind resources to the event loop that is active when the resource is first used or initialized. Creating the client outside of an async context may lead to issues if the client inadvertently attempts to bind to an event loop that is not yet started or is not the intended loop for its operations. 3. Testing and Best Practices: Best practice dictates that the AsyncClient should be instantiated within the lifecycle of your application's event loop—typically within an async function, a startup/shutdown handler (if using a framework like FastAPI), or a fixture (if using pytest-asyncio). This ensures the client is tied to a valid event loop and is correctly closed after use [3][2]. In summary, to avoid unexpected runtime errors, resource leakage, and event loop incompatibility, you should initialize your httpx.AsyncClient inside an async function or within an appropriate lifecycle hook, rather than at the module level [3][2].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the relevant file and locate any AsyncClient lifecycle handling.
git ls-files 'backend/app/agents/pulse.py' 'backend/app/**' | sed -n '1,20p'
printf '\n--- pulse.py ---\n'
cat -n backend/app/agents/pulse.py

printf '\n--- AsyncClient references ---\n'
rg -n "AsyncClient|aclose\(|async with .*httpx|httpx" backend/app -S

Repository: welshDog/HyperCode-V2.4

Length of output: 10815

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- references to backend/app/agents/pulse.py symbols ---'
rg -n "\bpulse\b|\b_http\b|PulseAgent" backend/app -S

printf '\n%s\n' '--- main.py shutdown section ---'
cat -n backend/app/main.py | sed -n '180,250p'

printf '\n%s\n' '--- pulse.py surrounding context ---'
cat -n backend/app/agents/pulse.py

Repository: welshDog/HyperCode-V2.4

Length of output: 8713

Close the shared AsyncClient on shutdown. The module-level client is fine to construct, but it is never aclose()d, so the connection pool stays open for the process lifetime. Wire it into app lifespan teardown or close it explicitly.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/agents/pulse.py` at line 14, The module-level `_http`
`httpx.AsyncClient` in `pulse.py` is created once but never closed, so its
connection pool can remain open for the process lifetime. Update the app
shutdown/lifespan teardown path to call `_http.aclose()` explicitly, or wire
`_http` into the existing lifespan cleanup alongside the other app resources so
it is always closed when the app stops.


Comment on lines +2 to +15

@coderabbitai coderabbitai Bot Jul 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Unit tests will break: they still mock pulse_mod.requests.get.

The removal of import requests (replaced by import httpx) means pulse_mod.requests no longer exists. All three tests in backend/tests/unit/test_agent_pulse.py call monkeypatch.setattr(pulse_mod.requests, "get", fake_get), which will raise AttributeError at collection time. Additionally, even if patched, the mock would not intercept the new await _http.get(...) path.

The tests must be migrated to mock _http.get (or inject a client) instead.

🔧 Example test migration pattern 
 # In test_agent_pulse.py

-    def fake_get(url: str, params: dict, timeout: int):
-        return _Response(200, {...})
-
-    monkeypatch.setattr(pulse_mod.requests, "get", fake_get)
+    async def fake_get(url, params=None):
+        return _Response(200, {...})
+
+    monkeypatch.setattr(pulse_mod._http, "get", fake_get)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@backend/app/agents/pulse.py` around lines 2 - 15, The pulse agent switched
from requests to a shared httpx.AsyncClient, so the existing tests that patch
pulse_mod.requests.get will fail because requests no longer exists and the new
path uses _http.get. Update the unit tests in test_agent_pulse.py to mock
pulse_mod._http.get (or otherwise inject a client) so they intercept the awaited
HTTP call in pulse.process/brain usage and avoid AttributeError during
collection.

class PulseAgent:
def __init__(self):
self.brain = brain
Expand All @@ -16,8 +25,10 @@ async def process(self, payload=None, conversation_id: str | None = None):
# 1. Grab raw vitals from Prometheus (Checking what services are 'up')
try:
# Using 'up' query to see which targets are up
# This query returns status of all scraped targets
res = requests.get(self.prometheus_url, params={'query': 'up'}, timeout=5)
# This query returns status of all scraped targets.
# Awaited, not sync: this runs on hypercode-core's event loop, and a
# blocking call here delays every request the API is serving.
res = await _http.get(self.prometheus_url, params={'query': 'up'})
if res.status_code == 200:
raw_data = res.json()
# Extract relevant info to keep prompt size down
Expand Down
Loading