fix(tui_gateway): route setup.runtime_check and setup.status to RPC pool (#50005) - #165
fix(tui_gateway): route setup.runtime_check and setup.status to RPC pool (#50005)#165hashbender wants to merge 1 commit into
Conversation
|
Review Complete Files Reviewed: 2 By Severity:
Minor cosmetic issue: inconsistent comment alignment in a test file. The PR's core GIL starvation fix for tui_gateway/server.py looks correct. Files Reviewed (2 files) |
There was a problem hiding this comment.
Risk: 🟢 Low (8/100) — 1 low finding · 18 LOC across 2 files
Summary
A single low-severity style finding was identified:
finding-001: Comment alignment inconsistency
- File:
tests/tui_gateway/test_inline_rpc_gil_starvation.py(line 70) - The
FRONTEND_POLLED_RPCSlist has most entries aligned with 10 spaces before the#comment marker, but the newly-addedsetup.runtime_checkentry uses only 3 spaces, breaking the visual columnar alignment. - No functional impact — purely cosmetic.
PR Assessment
The PR targets GIL starvation in tui_gateway/server.py by introducing inline handling for fast/simple RPCs and routing long-running handlers to a thread pool. The architectural approach is sound and well-motivated by desktop sidebar polling behavior. No correctness, concurrency, or security issues were found in the core change.
| "session.list", # loads session list — SQLite query | ||
| "pet.info", # petdex poll — file/network read | ||
| "process.list", # background process status — process registry scan | ||
| "setup.runtime_check", # runtime readiness — resolve_runtime_provider() I/O |
There was a problem hiding this comment.
🟢 Inconsistent comment alignment in FRONTEND_POLLED_RPCS list (style)
In tests/tui_gateway/test_inline_rpc_gil_starvation.py, the FRONTEND_POLLED_RPCS list entries are aligned with 10 spaces between the closing quote and the # comment marker, except for the newly-added setup.runtime_check entry on line 70 which has only 3 spaces. This is purely cosmetic and does not affect functionality, but violates the established alignment pattern in the same list block.
💡 Suggestion: Add 7 more spaces before the # on line 70 to match the 10-space alignment of the other entries in the list (lines 67, 68, 69, 71).
| "setup.runtime_check", # runtime readiness — resolve_runtime_provider() I/O | |
| "setup.runtime_check", # runtime readiness — resolve_runtime_provider() I/O |
📋 Prompt for AI Agents
In tests/tui_gateway/test_inline_rpc_gil_starvation.py on line 70, the comment alignment for the setup.runtime_check entry has only 3 spaces before the # while all other list entries have 10 spaces. Change the line to use 10 spaces before the # to align with the rest of the FRONTEND_POLLED_RPCS list.
Summary
setup.runtime_checkandsetup.statusare polled by the Desktop frontend on connect and periodically viaevaluateRuntimeReadiness()(use-status-snapshot.ts). Both were missing from_LONG_HANDLERS, so they ran inline on the WS reader thread.Under GIL pressure from concurrent agent turns (terminal I/O, large output processing, background process completions), either RPC can take seconds:
setup.runtime_checkcallsresolve_runtime_provider()— reads config, checks auth state, may probe the provider endpointsetup.statuscalls_has_any_provider_configured()— scans provider config + credential filesWhile either blocks the reader thread, the WS read loop cannot service subsequent requests. The frontend's 120s RPC timeout fires, the client closes the WebSocket, and
setup.runtime_check's response is lost —interpretRuntimeReadiness()gets no signal, returnsready=false, and the status bar shows "needs setup" even though the provider is correctly configured.This is the same failure mode described in NousResearch#50005. PR NousResearch#55545 added
session.list,pet.info, andprocess.listto_LONG_HANDLERSbut missed these two.Fix
Add
setup.runtime_checkandsetup.statusto_LONG_HANDLERSintui_gateway/server.py.dispatch()now returns immediately (_pool.submit+return None) for both, keeping the WS read loop free under GIL pressure.Tests
Added both RPCs to
FRONTEND_POLLED_RPCSintests/tui_gateway/test_inline_rpc_gil_starvation.py— the parametrizedtest_frontend_polled_rpc_is_pool_routednow asserts all 5 frontend-polled RPCs are in_LONG_HANDLERS.Relationship to other PRs
session.list/pet.info/process.listto_LONG_HANDLERS+ WS ping relax + token coalescingconfigured=truewhenchecksDisagree=true(transient outage mitigation)This PR is the backend root-cause fix for the remaining gap. NousResearch#55707 is a complementary frontend mitigation that handles transient provider outages where
setup.runtime_checkreturnsok: false(not a transport failure). Both are useful; neither subsumes the other.Mirror-of: NousResearch#56084
NousResearch#56084