fix(tui): keep exec quick commands off the RPC reader

[frizikk]

What does this PR do?

Fixes #67627.

command.dispatch ran quick commands of type: exec (which call subprocess.run) on the TUI gateway RPC reader. A slow command therefore prevented subsequent fast RPCs from being read. This routes only resolved exec quick commands to the existing worker pool; aliases and other command-dispatch paths remain inline to retain their ordering behavior.

Related Issue

Fixes #67627

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• tui_gateway/server.py
  • Dynamically routes command.dispatch to the existing worker pool only when its resolved quick command is an exec command.
• tests/tui_gateway/test_protocol.py
  • Covers a blocking exec quick command while a fast RPC still completes, its eventual emitted response, and an inline non-exec alias control case.

How to Test

1. Define a quick command with type: exec that blocks briefly; dispatch it through the TUI gateway.
2. Dispatch a fast RPC before it completes; it returns without waiting for the command subprocess.
3. Verify the exec command response is emitted once the process completes.

Commands run on Linux (CachyOS):

uv sync --extra all --extra dev
.venv/bin/python -m pytest tests/tui_gateway/test_protocol.py -q
.venv/bin/python -m ruff check tui_gateway/server.py tests/tui_gateway/test_protocol.py
git diff --check origin/main...HEAD

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass — focused TUI protocol coverage was run; not the entire suite.
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Linux (CachyOS)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — N/A; no user-facing contract changed.
• I've updated cli-config.yaml.example if I added/changed config keys — N/A; no configuration keys changed.
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A; no contributor workflow changed.
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — uses the existing Python worker pool and subprocess.run path; no platform-specific code added.
• I've updated tool descriptions/schemas if I changed tool behavior — N/A; RPC schema is unchanged.

For New Skills

N/A — this PR does not add a skill.

Screenshots / Logs

• Refreshed exact candidate 9d6eee45436f8ec7b26debbea2d30e5114d0522c is patch-equivalent to the original commit and rebased onto upstream main at 936dd7346fd7fd8107af1ce7fc019c07c001c1bd (2026-08-11).
• pytest tests/tui_gateway/test_protocol.py -q — 45 passed on the refreshed tree.
• Ruff on all three changed Python files — All checks passed.
• Windows-footgun scan on all changed files — no findings.
• git diff --check and conflict-marker/merge-tree checks — passed.
• Contributor attribution remains mapped to the GitHub account; rebase changed only the committer metadata.

[teknium1]

Thanks for the focused regression fix. The premise is confirmed on current upstream main: dispatch() sends only _LONG_HANDLERS to the pool (tui_gateway/server.py:1499), while command.dispatch synchronously reaches the exec quick-command subprocess.run(..., timeout=30) branch (tui_gateway/server.py:12914-12937).

The patch reuses the established pool/transport path, limits asynchronous routing to resolved type: exec quick commands, and covers both non-blocking exec behavior and inline aliases in tests/tui_gateway/test_protocol.py:1905.

Automated hermes-sweeper review.

[PRATHAMESH75]

Reviewed against upstream/main. This correctly fixes #67627.

Root cause matches. dispatch() (tui_gateway/server.py:1499) offloads only method not in _LONG_HANDLERS, so command.dispatch runs inline on the RPC reader — and its type: exec branch calls subprocess.run(..., timeout=30) (tui_gateway/server.py:12931), blocking the reader for up to 30s. That is exactly the reported defect.

Fix is well-scoped. _is_pool_routed_request pool-routes command.dispatch only when the resolved quick command is type: exec; aliases and every other slash path stay inline, preserving ordering for session-affecting commands. The name resolution in the gate (_resolve_name(params.get("name","").lstrip("/"))) matches the handler's own resolution (server.py:12916), so routing and execution agree on the resolved name. The try/except -> False fallback means a config-load failure degrades to the old inline behavior rather than dropping the request.

Coverage is adequate. test_dispatch_quick_command_exec_does_not_block_fast_handler proves the exec dispatch returns None immediately and a concurrent fast.ping completes before the subprocess is released; test_dispatch_non_exec_quick_command_stays_inline pins the alias-stays-inline invariant. Both align with the issue's expected behavior.

Nit (non-blocking): the gate re-runs _load_cfg()/_resolve_name() that the handler runs again a moment later — one extra cheap config read per exec dispatch, not worth complicating the routing path over.

LGTM from an automated triage pass.

[GottZ]

This was generated by AI during triage.

Summary

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

Related pull requests

• fix(tui): keep exec quick commands off the RPC reader #67628 best fix — (+86/-1) — n/a: Reuses _is_pool_routed_request() and the existing pool/context-snapshot path for resolved type: exec commands while preserving inline non-exec ordering; focused tests cover reader responsiveness, eventual exec output, and inline aliases. The maintainer-bot keep_open review and the recorded best-fix finding support retaining this implementation and coverage.
• fix(tui): pool-route exec quick commands to unblock RPC reader (#67627) #67789 duplicate — (+23/-0) — n/a: Pool-routes the same resolved exec-command path but duplicates the pool-submission block and adds no regression tests. Despite its maintainer-bot keep_open review, that review records salvageability=none and explicitly identifies fix(tui): pool-route exec quick commands to unblock RPC reader (#67627) #67789 as functionally duplicate of fix(tui): keep exec quick commands off the RPC reader #67628; the 184-commit contributor comment reaches the same conclusion.

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 preserving 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
    P67628 -->|best fix| I67627
    class I67627 open
    class P67628 open
    class P67789 open
    class P67628 best
    class P67628 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.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(tui): keep exec quick commands off the RPC reader — moving exec quick commands to the pool with a redacted timeout error is the right shape. Observations:

1. tui_gateway/server.py _is_pool_routed_request — it calls _load_cfg() on every command.dispatch; if _load_cfg raises (corrupt config), the broad except Exception: return False silently routes the exec command back INLINE on the reader thread — the exact blocking bug this PR fixes — with no log. Consider routing to the pool on config-read failure, or at least logging the fallback.

2. Classification TOCTOU: dispatch classifies from cfg, but the pool worker re-reads cfg in methods_tools.py. A config edit between the two can flip the classification (e.g. dispatch sees alias → inline on reader, cfg flips to exec → blocking on the reader thread). Narrow window, but passing the resolved quick-command entry to the worker (or classifying once) would close it.

3. Client contract: for exec quick commands dispatch() now returns None and the response arrives asynchronously from the pool. Verify every caller of command.dispatch (desktop slash.exec, TUI _SlashWorker, web dashboard) already handles a deferred/null response — a synchronous expectation would break exec quick commands client-side.