Skip to content

fix(tui): pool-route exec quick commands to unblock RPC reader (#67627) - #67789

Open
JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/gemini-functioncall-preserve
Open

JonthanaHanh wants to merge 1 commit into
NousResearch:mainfrom
JonthanaHanh:fix/gemini-functioncall-preserve

Conversation

@JonthanaHanh

Copy link
Copy Markdown
Contributor

Fix

Route command.dispatch with type: exec quick commands to the thread pool so the RPC reader loop stays free while the subprocess runs (up to 30s timeout).

Problem

command.dispatch runs synchronously on the TUI gateway RPC reader thread. When a quick command has type: exec, the handler calls subprocess.run(..., timeout=30) inline. During this time, fast RPCs like session.interrupt or approval.respond sit unread in the stdin pipe, making the TUI appear frozen.

Root cause

dispatch() only offloads methods listed in _LONG_HANDLERS. command.dispatch is not in that set because most quick commands (aliases, slash commands) need inline ordering. But exec-type quick commands block the reader unnecessarily.

Fix details

In dispatch(), before the _LONG_HANDLERS check, detect command.dispatch calls targeting exec-type quick commands and route them to the pool. Non-exec quick commands remain inline to preserve ordering.

The detection loads quick_commands from config and checks the resolved command's type. If type == "exec", the handler runs on the pool; otherwise it falls through to normal inline dispatch.

Fixes #67627

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of open #67628: both pool-route only resolved exec quick commands from command.dispatch, leaving non-exec command ordering inline. #67628 is earlier and includes focused reader-blocking regression coverage.

@frizikk

frizikk commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Thanks for working on this. I compared both implementations after the triage note. #67628 predates this PR, covers the same resolved type: exec path through the existing shared pool-routing machinery, and includes focused regression tests proving that the reader stays responsive while non-exec aliases remain inline. Its required CI is green.

To avoid maintaining two equivalent fixes, I agree that this PR should be closed as a duplicate in favor of #67628.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused TUI responsiveness fix. The premise is confirmed on current main: command.dispatch stays inline in tui_gateway/server.py:1649, then its exec quick-command branch synchronously calls subprocess.run(..., timeout=30) at tui_gateway/server.py:14777-14797.

Problems

  • This is functionally duplicate of earlier open fix(tui): keep exec quick commands off the RPC reader #67628. Its patch uses the shared _is_pool_routed_request decision point and includes targeted reader-responsiveness and non-exec-alias ordering regression tests; this PR changes only tui_gateway/server.py.
  • Current generic pool-routing coverage at tests/tui_gateway/test_protocol.py:1983-2112 does not exercise this command.dispatch exec path, and this PR adds no replacement coverage.

Suggested changes

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Two open PRs address Issue #67627 by pool-routing resolved exec quick commands so subprocess execution no longer blocks the RPC reader. #67628 reuses the shared routing path and adds focused exec-responsiveness and non-exec-alias tests, while #67789 implements equivalent routing through a duplicated submission block without corresponding tests.

Related pull requests

Duplicates

#67789 is a functional duplicate of #67628: both offload only resolved exec quick commands, while #67628 uses the shared routing predicate and includes focused regression coverage.

Suggested consolidation

Keep #67628 open with a salvage path: preserve its shared routing predicate and focused exec/non-exec regression tests. Close #67789 as duplicate of #67628 because its equivalent implementation duplicates pool-submission logic and contributes no distinct tested behavior.

Complex graph

flowchart 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
    I67627(["issue #67627 (open)"])
    subgraph Dup67628 ["PRs duplicating each other"]
        P67628["PR #67628 (open)"]
        P67789["PR #67789 (open)"]
    end
    P67789 -->|fixes| I67627
    class I67627 open
    class P67628 open
    class P67789 open
    class P67628 best
    class P67789 target
    click I67627 "https://github.com/NousResearch/hermes-agent/issues/67627"
    click P67628 "https://github.com/NousResearch/hermes-agent/pull/67628"
    click P67789 "https://github.com/NousResearch/hermes-agent/pull/67789"
Loading

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 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 6 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (4 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(tui): exec quick commands block the RPC reader loop

5 participants