refactor(hermes_cli/web_server): migrate asyncio.get_event_loop() to get_running_loop() by Kewe63 · Pull Request #1 · Kewe63/hermes-agent

Kewe63 · 2026-06-20T21:02:20Z

Summary

Single remaining deprecated call at _start_device_code_flow L6173 (MiniMax PKCE device-flow boot). Outer function is async; get_running_loop() is the direct drop-in replacement. Inner closure _do_minimax_request is correctly sync — run_in_executor bridges from the async caller to a thread-pool worker. get_running_loop() is only invoked from the async caller, so the deprecation-free replacement is safe.

Note: this file already uses asyncio.get_running_loop() in 8 other places (Nous device flow was fixed separately). This PR cleans up the last deprecated site (MiniMax PKCE).

   device_data = await asyncio.get_event_loop().run_in_executor(

   device_data = await asyncio.get_running_loop().run_in_executor(
       None, _do_minimax_request
   )

Tests

New tests/hermes_cli/test_web_server_event_loop_migration.py (3 tests, all passing):

test_get_event_loop_removed — pins zero remaining deprecated sites in this file
test_migrated_site_in_async_def — site sits inside async def _start_device_code_flow (walks past nested sync closures like _do_minimax_request to find the outer async def)
test_time_module_already_imported — guards against NameError if import time is dropped

uv run --frozen pytest tests/hermes_cli/test_web_server_event_loop_migration.py -v → 3 passed in 0.19s

ruff check and ruff format clean on both files.

Files Changed

hermes_cli/web_server.py | 2 +-
tests/hermes_cli/test_web_server_event_loop_migration.py | 132 +++++++++++++ (new)

Per-File Split (memory rule)

Following fix/lsp-client-time-monotonic-deprecation. Remaining files in the deprecation sweep:

File	Status
plugins/platforms/photon/adapter.py (3 hits)	sync+async mixed; needs careful audit
plugins/platforms/simplex/adapter.py (1 hit)	needs sync/async audit

These will follow in separate PRs after this one merges.

No issue filed upstream — code-hunt find (file-area coverage analysis flagged hermes_cli/web_server.py as a high-density deprecated-API zone).

…get_running_loop() Single remaining deprecated call at _start_device_code_flow L6173 (MiniMax PKCE device-flow boot). Outer function is async; get_running_loop() is the direct drop-in replacement. Inner closure _do_minimax_request is correctly sync — run_in_executor bridges from the async caller to a thread-pool worker. get_running_loop() is only invoked from the async caller, so the deprecation-free replacement is safe. Note: this file already uses asyncio.get_running_loop() in 8 other places (Nous device flow was fixed separately). This PR cleans up the last deprecated site (MiniMax PKCE). Tests pin: - zero remaining asyncio.get_event_loop() sites in this file; - the migrated site sits inside async def _start_device_code_flow (walked past nested sync closures to the outer async def); - the import time is still present (used by time.time() for expires_at math). No production behaviour change. Per memory rule: per-file split. Following fix/lsp-client-time-monotonic-deprecation. Remaining files in the deprecation sweep will follow in separate PRs.

greptile-apps

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

qodo-code-review · 2026-06-20T21:03:09Z

PR Summary by Qodo

Replace deprecated asyncio.get_event_loop() in MiniMax device flow
✨ Enhancement 🧪 Tests 🕐 10-20 Minutes

Description

• Replace deprecated asyncio.get_event_loop() with get_running_loop() in MiniMax PKCE device flow.
• Preserve existing run_in_executor bridge from async flow to sync httpx request closure.
• Add regression tests to pin the migration and prevent deprecation reintroduction.

Diagram

graph TD
  T["test_web_server_event_loop_migration.py"] --> WS["web_server.py"] --> F(["async _start_device_code_flow"]) --> L["get_running_loop()"] --> E["run_in_executor"] --> W["thread worker"] --> X{{"MiniMax OAuth API"}}

  subgraph Legend
    direction LR
    _file["File"] ~~~ _async(["Async function"]) ~~~ _ext{{"External"}}
  end

High-Level Assessment

The following are alternative approaches to this PR:

1. Use asyncio.to_thread() for the sync MiniMax request

➕ Avoids explicit event loop access entirely (no get_*_loop calls)
➕ More readable for the common 'run sync code in a thread' case
➖ Slightly broader change than a targeted deprecation fix
➖ Behavior depends on Python version support and executor configuration expectations

2. Wrap executor usage behind a shared helper (e.g., run_sync_io(fn))

➕ Centralizes async↔sync bridging patterns and future migrations
➕ Reduces repetition across other provider flows
➖ Introduces an abstraction for a single remaining call site
➖ May delay merging a focused deprecation cleanup

Recommendation: Keep the PR’s minimal swap to asyncio.get_running_loop() as the safest, behavior-preserving deprecation fix. Consider a follow-up refactor to asyncio.to_thread() or a shared helper only if you plan a broader cleanup across remaining get_event_loop sites in other modules.

Files changed (2) +133 / -1

Refactor (1) +1 / -1

web_server.pyMigrate MiniMax executor scheduling to asyncio.get_running_loop() +1/-1

Migrate MiniMax executor scheduling to asyncio.get_running_loop()

• Replaces the deprecated asyncio.get_event_loop() call with asyncio.get_running_loop() for the MiniMax PKCE device-code boot path. Keeps the synchronous _do_minimax_request executed via run_in_executor from the enclosing async function.

hermes_cli/web_server.py

Tests (1) +132 / -0

test_web_server_event_loop_migration.pyAdd regression tests for event loop API migration +132/-0

Add regression tests for event loop API migration

• Adds tests that assert no asyncio.get_event_loop() remains in hermes_cli/web_server.py and that the migrated run_in_executor call is enclosed by async def _start_device_code_flow. Also asserts time remains imported to prevent regressions in expires_at computations.

tests/hermes_cli/test_web_server_event_loop_migration.py

github-actions · 2026-06-20T21:03:17Z

🔎 Lint report: `fix/web-server-event-loop-clean` vs `origin/main`

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11279 on HEAD, 11279 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 5911 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

qodo-code-review · 2026-06-20T21:05:07Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (1)

Context used

✅ Compliance rules (platform): 120 rules

✅ Skills: software-development, github

1. Source-inspection tests added 📜 Skill insight ▣ Testability

Description

The new regression tests validate the event-loop migration by reading and parsing
hermes_cli/web_server.py as raw text and asserting on exact strings/lexical structure, which makes
them brittle to harmless refactors that preserve behavior. This conflicts with the requirement that
tests verify observable behavior rather than internal implementation details and can cause false CI
failures and extra maintenance.

Code

tests/hermes_cli/test_web_server_event_loop_migration.py[R34-132]

+REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+WEB_SERVER = REPO_ROOT / "hermes_cli" / "web_server.py"
+
+
+def _read_web_server() -> str:
+    assert WEB_SERVER.is_file(), f"missing {WEB_SERVER}"
+    return WEB_SERVER.read_text(encoding="utf-8")
+
+
+def test_get_event_loop_removed() -> None:
+    """No ``asyncio.get_event_loop()`` should remain in
+    hermes_cli/web_server.py — the deprecation must be fully resolved
+    in this file before siblings follow."""
+    content = _read_web_server()
+    matches = re.findall(r"asyncio\.get_event_loop\(", content)
+    assert matches == [], (
+        f"Found {len(matches)} remaining `asyncio.get_event_loop(` "
+        f"call(s) in hermes_cli/web_server.py. The 3.10+ deprecation "
+        f"must be fully resolved."
+    )
+
+
+def _find_outer_async_def(
+    content: str, line_idx: int, max_lookback: int = 500
+) -> tuple[int | None, str | None]:
+    """Walk backwards from ``line_idx`` to find the *outer* ``async def``.
+    Inner ``def`` (sync closures) are skipped — they are nested
+    structural blocks but the loop-policy question is decided by the
+    nearest enclosing async def.
+
+    Returns (line_number_1based, function_name) or (None, None) if not
+    found within ``max_lookback`` lines.
+    """
+    lines = content.split("\n")
+    for i in range(line_idx - 1, max(0, line_idx - max_lookback), -1):
+        m = re.match(r"^(\s*)(async )?def (\w+)", lines[i])
+        if m:
+            async_kw = m.group(2)
+            name = m.group(3)
+            if async_kw:
+                return i + 1, name
+    return None, None
+
+
+def test_migrated_site_in_async_def() -> None:
+    """The migrated ``asyncio.get_running_loop().run_in_executor(...)``
+    call site at the MiniMax PKCE device-flow boot must sit inside
+    ``async def _start_device_code_flow``. If the call moves to a
+    sync caller the call would raise RuntimeError because there is
+    no running loop in a sync context."""
+    content = _read_web_server()
+    lines = content.split("\n")
+
+    # The migrated site is multi-line: `await ... .run_in_executor(`
+    # spans two lines, and ``_do_minimax_request`` is the function arg
+    # on the next line. Walk forward from each ``run_in_executor(`` start
+    # and accept the call if a MiniMax marker appears in the next 5
+    # lines.
+    migrated_sites = []
+    for i, line in enumerate(lines):
+        if "await asyncio.get_running_loop().run_in_executor(" not in line:
+            continue
+        ctx = "\n".join(lines[i : min(i + 6, len(lines))])
+        if "minimax" not in ctx.lower():
+            continue
+        migrated_sites.append(i)
+    assert len(migrated_sites) >= 1, (
+        "expected at least one `await asyncio.get_running_loop().run_in_executor` "
+        "site whose next 5 lines mention `minimax` (the MiniMax PKCE boot — "
+        "the only deprecation fix in this PR)."
+    )
+
+    idx = migrated_sites[0]
+    owner_line, owner_name = _find_outer_async_def(content, idx)
+    assert owner_name is not None and owner_line is not None, (
+        f"migrated MiniMax site at line {idx + 1} has no outer async def "
+        f"within 500 lines. _start_device_code_flow is async — this "
+        f"site must live inside it."
+    )
+    assert owner_name == "_start_device_code_flow", (
+        f"migrated MiniMax site at line {idx + 1} is inside `{owner_name}()`, "
+        f"expected `_start_device_code_flow`."
+    )
+    decl = lines[owner_line - 1]
+    assert decl.lstrip().startswith("async def"), (
+        f"{owner_name} at line {owner_line} is not declared async: {decl.rstrip()!r}"
+    )
+
+
+def test_time_module_already_imported() -> None:
+    """Sanity guard: ``time`` is used elsewhere in this file
+    (e.g. time.time() at line ~6091 in _start_device_code_flow) so
+    the import should already exist."""
+    content = _read_web_server()
+    assert re.search(r"^import time\b", content, re.MULTILINE), (
+        "hermes_cli/web_server.py no longer imports `time` — the "
+        "device-code flow uses time.time() to compute expires_at; "
+        "removing the import would crash the flow."
+    )

Evidence

PR Compliance ID 980851 requires tests to validate observable behavior rather than internal
implementation details, but the added tests open hermes_cli/web_server.py from disk and hard-code
exact source substrings for both the removed API (asyncio.get_event_loop) and the replacement call
(e.g., await asyncio.get_running_loop().run_in_executor(...)), and even infer the enclosing async
function via a simple backward regex scan instead of parsing Python structure. Because the
production code places the call inside async def _start_device_code_flow, any semantically
equivalent refactor (such as assigning loop = asyncio.get_running_loop() and then awaiting
loop.run_in_executor(...), or reformatting/moving code) would still satisfy the behavioral goal
but could fail these substring/format-dependent assertions.

tests/hermes_cli/test_web_server_event_loop_migration.py[34-132]
tests/hermes_cli/test_web_server_event_loop_migration.py[43-120]
hermes_cli/web_server.py[6045-6054]
hermes_cli/web_server.py[6160-6176]
Skill: software-development

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The current regression tests assert the event-loop deprecation migration by reading `hermes_cli/web_server.py` as text and matching exact strings/structure (including substring checks and a non-AST regex scan to determine the enclosing async function). This couples the tests to formatting and specific implementation choices, so benign refactors (e.g., storing the running loop in a local variable before calling `run_in_executor`) can break CI even when behavior is unchanged, and it also violates the requirement that tests verify observable behavior rather than internal implementation details.

## Issue Context
Compliance requires tests to assert on observable behavior (outputs, warnings/exceptions, state changes) instead of internal implementation details. The real invariants being guarded are semantic: `asyncio.get_event_loop()` should not be invoked in `hermes_cli/web_server.py`, and the `run_in_executor` usage should be driven by a *running* loop from within an async context (currently within `_start_device_code_flow`), without the test being dependent on exact source formatting.

## Fix Focus Areas
- tests/hermes_cli/test_web_server_event_loop_migration.py[34-132]
- hermes_cli/web_server.py[6045-6054]
- hermes_cli/web_server.py[6160-6176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

qodo-code-review · 2026-06-20T21:05:08Z

+REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+WEB_SERVER = REPO_ROOT / "hermes_cli" / "web_server.py"
+
+
+def _read_web_server() -> str:
+    assert WEB_SERVER.is_file(), f"missing {WEB_SERVER}"
+    return WEB_SERVER.read_text(encoding="utf-8")
+
+
+def test_get_event_loop_removed() -> None:
+    """No ``asyncio.get_event_loop()`` should remain in
+    hermes_cli/web_server.py — the deprecation must be fully resolved
+    in this file before siblings follow."""
+    content = _read_web_server()
+    matches = re.findall(r"asyncio\.get_event_loop\(", content)
+    assert matches == [], (
+        f"Found {len(matches)} remaining `asyncio.get_event_loop(` "
+        f"call(s) in hermes_cli/web_server.py. The 3.10+ deprecation "
+        f"must be fully resolved."
+    )
+
+
+def _find_outer_async_def(
+    content: str, line_idx: int, max_lookback: int = 500
+) -> tuple[int | None, str | None]:
+    """Walk backwards from ``line_idx`` to find the *outer* ``async def``.
+    Inner ``def`` (sync closures) are skipped — they are nested
+    structural blocks but the loop-policy question is decided by the
+    nearest enclosing async def.
+
+    Returns (line_number_1based, function_name) or (None, None) if not
+    found within ``max_lookback`` lines.
+    """
+    lines = content.split("\n")
+    for i in range(line_idx - 1, max(0, line_idx - max_lookback), -1):
+        m = re.match(r"^(\s*)(async )?def (\w+)", lines[i])
+        if m:
+            async_kw = m.group(2)
+            name = m.group(3)
+            if async_kw:
+                return i + 1, name
+    return None, None
+
+
+def test_migrated_site_in_async_def() -> None:
+    """The migrated ``asyncio.get_running_loop().run_in_executor(...)``
+    call site at the MiniMax PKCE device-flow boot must sit inside
+    ``async def _start_device_code_flow``. If the call moves to a
+    sync caller the call would raise RuntimeError because there is
+    no running loop in a sync context."""
+    content = _read_web_server()
+    lines = content.split("\n")
+
+    # The migrated site is multi-line: `await ... .run_in_executor(`
+    # spans two lines, and ``_do_minimax_request`` is the function arg
+    # on the next line. Walk forward from each ``run_in_executor(`` start
+    # and accept the call if a MiniMax marker appears in the next 5
+    # lines.
+    migrated_sites = []
+    for i, line in enumerate(lines):
+        if "await asyncio.get_running_loop().run_in_executor(" not in line:
+            continue
+        ctx = "\n".join(lines[i : min(i + 6, len(lines))])
+        if "minimax" not in ctx.lower():
+            continue
+        migrated_sites.append(i)
+    assert len(migrated_sites) >= 1, (
+        "expected at least one `await asyncio.get_running_loop().run_in_executor` "
+        "site whose next 5 lines mention `minimax` (the MiniMax PKCE boot — "
+        "the only deprecation fix in this PR)."
+    )
+
+    idx = migrated_sites[0]
+    owner_line, owner_name = _find_outer_async_def(content, idx)
+    assert owner_name is not None and owner_line is not None, (
+        f"migrated MiniMax site at line {idx + 1} has no outer async def "
+        f"within 500 lines. _start_device_code_flow is async — this "
+        f"site must live inside it."
+    )
+    assert owner_name == "_start_device_code_flow", (
+        f"migrated MiniMax site at line {idx + 1} is inside `{owner_name}()`, "
+        f"expected `_start_device_code_flow`."
+    )
+    decl = lines[owner_line - 1]
+    assert decl.lstrip().startswith("async def"), (
+        f"{owner_name} at line {owner_line} is not declared async: {decl.rstrip()!r}"
+    )
+
+
+def test_time_module_already_imported() -> None:
+    """Sanity guard: ``time`` is used elsewhere in this file
+    (e.g. time.time() at line ~6091 in _start_device_code_flow) so
+    the import should already exist."""
+    content = _read_web_server()
+    assert re.search(r"^import time\b", content, re.MULTILINE), (
+        "hermes_cli/web_server.py no longer imports `time` — the "
+        "device-code flow uses time.time() to compute expires_at; "
+        "removing the import would crash the flow."
+    )


1. Source-inspection tests added 📜 Skill insight ▣ Testability

The new regression tests validate the event-loop migration by reading and parsing hermes_cli/web_server.py as raw text and asserting on exact strings/lexical structure, which makes them brittle to harmless refactors that preserve behavior. This conflicts with the requirement that tests verify observable behavior rather than internal implementation details and can cause false CI failures and extra maintenance.

Agent Prompt

## Issue description The current regression tests assert the event-loop deprecation migration by reading `hermes_cli/web_server.py` as text and matching exact strings/structure (including substring checks and a non-AST regex scan to determine the enclosing async function). This couples the tests to formatting and specific implementation choices, so benign refactors (e.g., storing the running loop in a local variable before calling `run_in_executor`) can break CI even when behavior is unchanged, and it also violates the requirement that tests verify observable behavior rather than internal implementation details. ## Issue Context Compliance requires tests to assert on observable behavior (outputs, warnings/exceptions, state changes) instead of internal implementation details. The real invariants being guarded are semantic: `asyncio.get_event_loop()` should not be invoked in `hermes_cli/web_server.py`, and the `run_in_executor` usage should be driven by a *running* loop from within an async context (currently within `_start_device_code_flow`), without the test being dependent on exact source formatting. ## Fix Focus Areas - tests/hermes_cli/test_web_server_event_loop_migration.py[34-132] - hermes_cli/web_server.py[6045-6054] - hermes_cli/web_server.py[6160-6176]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

sonarqubecloud · 2026-06-20T21:07:07Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

…ture get_copilot_api_token now returns (api_token, base_url); the auth-remove suppression test still mocked it as a bare string, mis-unpacking into the credential-pool seed path and failing with 'No credential #1'.

…_id signature churn Two independent bugs evicted the cached gateway AIAgent on every turn, preventing the prompt cache from ever warming: 1. Model normalization mismatch: the post-run fallback-eviction check compared _agent.model (stripped in AIAgent.__init__) against the raw _resolve_gateway_model() config string. For vendor-prefixed config on native providers (e.g. 'deepseek/deepseek-v4-pro' vs 'deepseek-v4-pro') this was always unequal, so the agent was evicted after every successful run. Normalize _cfg_model the same way (skip aggregators). 2. Discord triggering message_id leaked into the cached system prompt via build_session_context_prompt()'s Discord IDs block. message_id changes every turn, so the agent-cache signature (computed from the ephemeral prompt) changed every Discord turn -> rebuild every message. The id is now injected per-turn into the user message (where per-turn content belongs and does not touch the cache signature); the cached IDs block carries a static pointer to it, preserving reply/react/pin via the discord tools. Adapted from NousResearch#28846. Bug #1 fix is the contributor's; bug NousResearch#2 reworked to be non-destructive (keeps the triggering-id capability instead of deleting it). Redundant auto-reset eviction (already on main via NousResearch#9893/NousResearch#48031) and the wrong-premise reset_context_note plumbing from the original PR were dropped. Co-authored-by: Hermes Agent <hermes@nousresearch.com>

greptile-apps Bot reviewed Jun 20, 2026

View reviewed changes

qodo-code-review Bot reviewed Jun 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(hermes_cli/web_server): migrate asyncio.get_event_loop() to get_running_loop()#1

refactor(hermes_cli/web_server): migrate asyncio.get_event_loop() to get_running_loop()#1
Kewe63 wants to merge 1 commit into
mainfrom
fix/web-server-event-loop-clean

Kewe63 commented Jun 20, 2026

Uh oh!

greptile-apps Bot left a comment

Uh oh!

qodo-code-review Bot commented Jun 20, 2026

Uh oh!

github-actions Bot commented Jun 20, 2026

Uh oh!

qodo-code-review Bot commented Jun 20, 2026

Uh oh!

qodo-code-review Bot Jun 20, 2026

Uh oh!

sonarqubecloud Bot commented Jun 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Kewe63 commented Jun 20, 2026

Summary

Tests

Files Changed

Per-File Split (memory rule)

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

qodo-code-review Bot commented Jun 20, 2026

PR Summary by Qodo

Uh oh!

github-actions Bot commented Jun 20, 2026

🔎 Lint report: fix/web-server-event-loop-clean vs origin/main

ruff

ty (type checker)

Uh oh!

qodo-code-review Bot commented Jun 20, 2026

Code Review by Qodo

Uh oh!

qodo-code-review Bot Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 20, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

🔎 Lint report: `fix/web-server-event-loop-clean` vs `origin/main`