fix(gateway): scope GET /api/config to config-only profile to unwedge the event loop (#67936) - #67944
Conversation
Related to #67936; this focused fix changes the config endpoint to the existing async-safe config-only profile scope. |
… event loop (NousResearch#67936) get_config() is an async handler on the event-loop thread but used the skills-aware _profile_scope(), which acquires the process-global _SKILLS_PROFILE_LOCK synchronously. A worker thread holding that lock (slow model/skills discovery) could then wedge the asyncio event loop, stalling HTTP/WebSocket responses and Desktop startup. It only resolves configuration through the task-local HERMES_HOME override, so switch to the config-only _config_profile_scope() already used by the sibling get_schema() handler. Add a regression test pinning that get_config() returns promptly while the skills lock is held.
9e5526a to
8c8e7bd
Compare
SummarySix PRs are associated with this two-issue complex. #45152 adds analytics cost presentation and profile-scoping regressions for #45148; #46426/#46432 scope dashboard-plugin routes; #48049/#57983 address cross-profile chat resume; and only #67944 changes the GET /api/config lock path reported by #67936. Related pull requests
Duplicates#46426 and #46432 substantially duplicate the dashboard-plugin profile-scoping change, with closed #46426 superseded by #46432. #48049 and #57983 overlap substantially on the chat-resume server path, but they are not clean duplicates because #57983 adds explicit frontend profile propagation and ambiguity-safe inference, whereas #48049 uses the contributor-reviewed unsafe first-match approach. Suggested consolidationKeep #67944 open with a salvage path: retain the focused _config_profile_scope() substitution and concurrent _SKILLS_PROFILE_LOCK regression test as the best existing fix for #67936. Do not fold #45152, #46432, #48049, or #57983 into this fix; #46426 is already closed as the duplicate predecessor of #46432, and no further duplicate closure is supported without reconciling the keep_open reviews and #48049's documented duplicate-session-ID flaw. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I67936(["issue #67936 (open)"])
P67944["PR #67944 (open)"]
P67944 -->|best fix| I67936
class I67936 open
class P67944 open
class P67944 best
class P67944 target
click I67936 "https://github.com/NousResearch/hermes-agent/issues/67936"
click P67944 "https://github.com/NousResearch/hermes-agent/pull/67944"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 6 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 75 kB of PR diffs, 19 kB of issue/PR text, 9 kB of discussion (16 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Closing this as superseded by upstream. The event-loop wedge on Closing in favor of the merged fix. Happy to reopen if anyone spots a gap the off-loop sweep didn't cover. |
What does this PR do?
GET /api/configis anasync defhandler that runs on the asyncioevent-loop thread, but it entered
_profile_scope(profile)— theskills-aware scope that acquires the process-global
_SKILLS_PROFILE_LOCKsynchronously. When a worker thread holds that lock (e.g. slow
model-option / skills discovery during Desktop setup), the event loop
blocks inside
get_config()and can no longer flush queuedJSON-RPC/WebSocket responses or serve HTTP probes — remote Desktop startup
times out at "Loading Hermes settings".
get_config()only resolves configuration through the task-localHERMES_HOMEoverride; it never touches the skills module globals that_SKILLS_PROFILE_LOCKprotects. The siblingget_schema()handlerdirectly below it already uses the await-safe, config-only
_config_profile_scope(). This alignsget_config()with that pattern.Related Issue
Fixes #67936
Type of Change
Changes Made
hermes_cli/web_server.py:get_config()now uses_config_profile_scope(profile)instead of_profile_scope(profile),so the async handler never waits on
_SKILLS_PROFILE_LOCK. Added acomment explaining the event-loop hazard.
tests/hermes_cli/test_web_server_profile_unification.py: newTestConfigEndpointNonBlockingregression — while another thread holds_SKILLS_PROFILE_LOCK,get_config()still returns promptly. On the old_profile_scope()path this blocked for the full lock-hold duration.How to Test
scripts/run_tests.sh tests/hermes_cli/test_web_server_profile_unification.py -q→ passes (includes the new
TestConfigEndpointNonBlocking::test_get_config_does_not_wait_on_skills_lock)._SKILLS_PROFILE_LOCKin a background thread and callasyncio.run(web_server.get_config())— returns in ~0.001s with the fixvs. the full hold duration before it.
Checklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/ACONTRIBUTING.mdorAGENTS.md— N/A